Remove development artifacts from the NPM package

[jessebeach]

Results in the following package

.npmignore
CHANGELOG.md
docs/
lib/
LICENSE.md
package.json
README.md

[ljharb]

As expressed in other threads, I think none of this should be excluded.

If flow users are seeing problems due to flaws in flow, can they not ignore node_modules/**/src/*, for example?

[gaearon]

This package is shipped by default with CRA. CRA posits zero config Flow integration (which worked before). The zero config-ness was broken by shipping Flow-annotated sources. Yes, people technically can work around this but it's a regression for our zero config promise. I realize ideally this should be solved at Flow level but it won't happen within the next few months (although the feature is in the pipeline). This seems like a practical compromise, and when Flow solves this we can revert.

[coveralls]

Coverage remained the same at 99.57% when pulling adf84cc on jessebeach:remove-src-from-package into 131a75d on evcohen:master.

[ljharb]

Is there a github issue tracking this flow fix I can follow?

Even if a temporary concession was made for this flow issue, we'd still only need to exclude flow and src, no?

[beefancohen]

@ljharb @jessebeach could this be a reasonable compromise:
pull src & all other superfluous files from the npm package and suggest users to point to github repo & version tag in their package.json if they wish to have src included? we can make it very clear in the docs that if they want to have src pulled down for better debugging/offline usage they can use https://github.com/evcohen/eslint-plugin-jsx-a11y/tree/v6.0.0 for v6 for instance

[ljharb]

@evcohen I could be convinced to do a temporary removal as @gaearon suggests, but your compromise sounds like it'd be a permanent change - and it doesn't solve the use case of "I went offline without knowing I'd need that code, and now I'm screwed"

[gaearon]

The feature is called lazy typechecking. Unfortunately I don't think Flow team uses GH to track feature development but I can create an issue if it helps. The feature is already available behind a flag but they expect to enable it by default some time this year.

If I understand correctly just excluding src alone should be enough.

[beefancohen]

@ljharb no, but that's a tradeoff that the consumer makes by saying that they need those things. they wouldn't get semver for free, but they'd have full offline support. it depends what they value more.

"devDependencies": {
  "eslint-plugin-jsx-a11y": "https://github.com/evcohen/eslint-plugin-jsx-a11y/tree/v6.0.0"
}

(or whatever the correct syntax is) gives you all of src at all times

[jessebeach]

I could be convinced to do a temporary removal as @gaearon suggests

@ljharb I'm fine putting it back in later once the updated Flow behavior ships. As I've mentioned before, I'm agnostic on this issue. I really just want to eliminate the ecosystem friction now.

[gaearon]

IMO it's pretty important to eliminate friction in a plugin like this. Developers often resist caring about a11y, and even small friction can make people hesitant about using the plugin or including it in other preset-like packages. I don't mean to feamonger but that's my experience anyway. People take out optional things that's causing them trouble even if the real fix is simple.

[coveralls]

Coverage remained the same at 99.57% when pulling 8505503 on jessebeach:remove-src-from-package into c121cd0 on evcohen:master.

[ljharb]

I'd still like to see this list of npmignore additions reduced to only adding src - @jessebeach, do you mind making that change before merging?

@evcohen, I don't think "choose semver or choose offline access to all relevant parts of the dep" is a reasonable choice to ask someone to make, nor satisfies the concerns I've addressed. I think "remove for now, add everything back later" is a much better choice than that.

@jessebeach and @gaearon's points about "let's reduce ecosystem friction for now" and "a11y linting is a more important thing to propagate" are valid, and I'm convinced that it's the right choice to make for the time being.

@gaearon if you'd mind filing a github issue for Flow to make lazy typechecking the default, then we can file an issue on this repo to revert this PR once that lands.

[beefancohen]

@ljharb that's fine, but for what it's worth, I think offline support is such an edge case and isn't worth the package bloat. I think better debugging support is a more valuable consequence of providing the full source code, and directly correlates to better bug reports and PRs from contributors.

You've said this before, but this is an npm problem, not a package author's problem. If we are willing to adjust our package because of an unsolved upstream issue, we should be willing to now (and you are, so thank you for your flexibility on the issue 😃 ).

[jessebeach]

I'd still like to see this list of npmignore additions reduced to only adding src - @jessebeach, do you mind making that change before merging?

surely. The flow dir should be excluded as well.

[jessebeach]

Package now includes:

__mocks__/
__tests__/
.babelrc
.eslintignore
.eslintrc
.npmignore
CHANGELOG.md
docs/
lib/
LICENSE.md
package.json
README.md
scripts/

[beefancohen]

@jessebeach feel free to merge
@gaearon do you need me to port this into v5 or can you upgrade to v6?

[jessebeach]

@evcohen that should be good enough for a v6.0.1

[coveralls]

Coverage remained the same at 99.57% when pulling f9d3ba9 on jessebeach:remove-src-from-package into c121cd0 on evcohen:master.

[beefancohen]

v6.0.1 published