prettier / eslint-config-prettier

Turns off all rules that are unnecessary or might conflict with Prettier.
MIT License
5.41k stars 254 forks source link

Error in Node v6 after updating `eslint-config-prettier` to v3.1.0 #57

Closed meeber closed 5 years ago

meeber commented 5 years ago

After updating from v3.0.1 to v3.1.0, I see the following behavior in Node v6 with its bundled npm (v3.10.10):

echo '{"rules": {}}' | ./node_modules/.bin/eslint-config-prettier-check

Unexpected error TypeError: Cannot read property 'filter' of undefined
    at processString (/opt/code/demo/node_modules/eslint-config-prettier/bin/cli.js:78:7)
    at getStdin.then.string (/opt/code/demo/node_modules/eslint-config-prettier/bin/cli.js:37:22)
    at process._tickCallback (internal/process/next_tick.js:109:7)

Updating npm and reinstalling eslint-config-prettier-check resolves the problem. For project's using Travis CI, adding the following to .travis.yml does the trick:

before_install:
- npm install -g npm@latest

The problem doesn't occur in Node v8 or v10 (which come bundled with newer versions of npm). The problem also doesn't occur with v3.0.1 or earlier of eslint-config-prettier, even when using Node v6 and its bundled npm v3.10.10.

The root of the problem is that the metadata for v3.1.0 in https://registry.npmjs.org/eslint-config-prettier doesn't include the files key, whereas the metadata for previous versions do. The exclusion of the files key from registry metadata is consistent across other projects starting in late August. I created an npm support topic asking if this change was intentional, but it didn't receive any responses.

The reason the lack of the files key in the registry metadata results in an error when eslint-config-prettier is installed via npm v3.10.10 is because older versions of npm generate node_modules/<package>/package.json based on the metadata from https://registry.npmjs.org/eslint-config-prettier (which is missing the files key). Newer versions of npm instead create that file based on the package.json inside of https://registry.npmjs.org/eslint-config-prettier/-/eslint-config-prettier-3.1.0.tgz (which includes the files key).

Note that this problem only presents itself when eslint-config-prettier is installed as a dependency inside of another project. That's why this project's Travis build for Node v6 isn't failing.

lydell commented 5 years ago

Thanks for the thorough investigation!

It feels like npm should fix this since it's breaking change. But perhaps there aren't that many packages that read "files" from their own package.json so they don't consider it much of an issue.

I guess we can switch to a fs.readdirSync-based approach instead in the CLI as a workaround.

  1. Do you think npm will fix it? Or is there anything we can do, like publishing a new version or something?
  2. Would you be interested in making a PR that works around the issue?
meeber commented 5 years ago

I agree that this is something that npm should either fix or document, but I can't even make an educated guess if they'll do so. I haven't come across any mention of the registry change in docs, change logs, issue trackers, or commit histories. I'm assuming the change hasn't had any impact on most projects.

I don't think publishing a new version will fix it, and I'm not aware of any other solutions that don't involve changing code. Personally, I'm fine with just updating my project's Travis config to update npm. Maybe that workaround combined with this issue documenting the problem is a sufficient response? If not, I'm happy to submit a PR switching to fs.readdirSync.

lydell commented 5 years ago

I’ve released 3.2.0. Hopefully it fixes the problem!