Eslint-plugin-jsx-a11y: Proposal: ignore unneeded files from npm package

[zpao]

I filed https://github.com/evcohen/jsx-ast-utils/issues/18 so figured I'd take quick look here too. It looks like that'll shave off ~60kb for a ~44gb saving over the last month.

I know many people like to include other things in their final packages but figured it's worth trying :)

Cheers, and thanks for the great work!

[ljharb]

The only truly unneeded files are CI config, git config, and yarn/npm config - I don't think that would be very large. Tests, raw source, and babel config are definitely needed.

[evcohen]

for devDependencies only, I presume @ljharb - if the only cost is network bandwidth, then i'm in favor of keeping.

thanks for suggesting @zpao 👍

[ljharb]

@evcohen npm explore foo && npm install && npm test should always work.

[zpao]

@ljharb I strongly disagree but I'm not going to die on that hill (and this isn't the place to have that metadiscussion anyway)

[evcohen]

haha @zpao even if it has no bearing on an application/library's bundle size? not sure i agree with

should always work

but, for devDependencies it seems reasonable to provide users with as much information as possible with what code they are actually working with, not just what is posted on github. there's almost always a disparity between what you've downloaded locally and what's on version control.

[jessebeach]

Tests, raw source, and babel config are definitely needed.

I hope I'm interpreting this comment correctly. Why is source needed for a module package published to NPM? NPM should contain the built module files that have passed lint and tests and that have already passed through transforms. There's no need to burden someone who wants to use the module with the src files which may require who knows what transformation to actually run.

[ljharb]

Because I want to be able to work with the raw source for debugging, documentation, and testing even when offline.

[jessebeach]

Because I want to be able to work with the raw source for debugging, documentation, and testing even when offline.

You can just clone the git repo for that, no? And then npm link from the module you're working in to the one that you want the source code to exist for.

[ljharb]

Only if i knew to do that in advance; and as @evcohen pointed out, the important code is not what is in git, it's what is installed by npm.

[jessebeach]

Well then I stand with @zpao in that I disagree, but I'm not going to die on that hill either =D

[ljharb]

I would love it if npm itself solved this problem, by providing a way to indicate which files are needed for tests, etc., versus which files are needed for production – then users could choose for themselves which subset to install. However, that is their problem to solve, not that of individual package authors.

[lencioni]

I would love it if npm itself solved this problem

@ljharb is there an issue tracking this idea?

[ljharb]

There is on npm's repo (i can't find it at the moment), but they have no plans to work on it anytime soon.

[jessebeach]

Can't reach consensus. Closing.

[evcohen]

what if we minified and bundled our build output, though, to lessen the total tarball size? seems like a fair compromise

[ljharb]

As long as the original source was still included, that seems reasonable - I'll be surprised if that actually reduces the size much tho, and that will make stack traces less readable and debugging harder :-/

[gaearon]

Now that src files include @flow annotations, all Create React App consumers get this error (#249) if they try to enable Flow:

node_modules/eslint-plugin-jsx-a11y/src/rules/media-has-caption.js:54
 54:         if (child.type !== 'JSXElement') {
                       ^^^^ property `type`. Property not found in
 54:         if (child.type !== 'JSXElement') {
                 ^^^^^ Node

While this particular error could probably be fixed somehow, issues like this will keep reoccurring if this package's Flow version doesn't match the user's Flow version.

The user could add eslint-plugin-jsx-a11y to [ignore] of their Flow config but this breaks zero-configuration approach of Create React App, and is just weird (why would you blacklist a build time only plugin that isn't even your direct dependency?)

I wonder what are your thoughts on a sustainable solution to this. Do we need a script that strips // @flow out of all source files on npm? Presumably this would also break the "treat node_modules as inspectable" approach that seems desirable in this thread.

The easiest fix would be to stop shipping src but seems like this is ruled out by lack of consensus.

[ljharb]

The sustainable solution is that Flow shouldn't be looking at files in node_modules that are never imported. I've discussed this with the Flow team in the past and they've indicated a willingness to fix this.

[gaearon]

This would be amazing!
@gabelevi Do you know if https://github.com/evcohen/eslint-plugin-jsx-a11y/issues/156#issuecomment-311703256 is reasonable to expect to land anytime this year?

[jessebeach]

@ljharb @evcohen @gaearon We should just take src out of the package. It has marginal upside and we keep running into issues because we're including development artifacts in the built package.

I'll propose another PR for this ASAP. We can roll a v6.0.1

[ljharb]

I still strongly disagree with that; if you'd like to reopen that discussion we can continue it, but my position hasn't changed.

Flaws in Flow's implementation aren't a justification for making changes in other parts of the ecosystem.

[jessebeach]

Flaws in Flow's implementation aren't a justification for making changes in other parts of the ecosystem.

I concede this. What benefit precludes us from making a change now though that reduces frictions in related projects? I don't understand why having src in the package is a must-have in this case. What will break or be impossible to accomplish if we take it out to remove this friction from Create React App? I'd rather we make a small change on our end that has little cost to us in order to play well in the ecosystem today.

[jessebeach]

Posted #277