Ignore parse errors in .json files under node_modules/

[Macil]

An issue I run into weirdly fequently (just ran into it again yesterday while adding Flow support to Kefir) is that some dependency or sub-dependency of a project contains purposefully malformed JSON files (some examples: bower-json/test/pkg-bower-json-malformed/bower.json, config-chain/test/broken.json, npmconf/test/fixtures/package.json), and Flow tries to parse them and reports errors.

I worked around it by adding some [ignore] lines to the .flowconfig, but I think it's bad when I have to configure something specifically to deal with a sub-dependency that I didn't even explicitly choose myself. I figured I'd go through and send a bunch of pull requests to get these modules to npmignore their test/ directories, but some of them are opinionated about putting their tests on npm, and some are in code freeze and not accepting pull requests (coincidentally these are 2/3 of the modules mentioned in #869 (comment); the commenter said they wanted to fix this, but without cooperation from those projects, the fix is going to have to come from Flow).

In the past, Flow had the similar/superset issue that it would try to parse all .js and .json files in the directory tree and it would report parse errors in all of them. The issue was lessened by Flow being changed to only do this with .js files that had the @flow comment and all .json files. Flow no longer assumes that all .js files under node_modules are parseable by it, but it still assumes that all .json files are.

This feature request can be seen as a much more restricted form of #869 that solves most of the issue that prompted that request.

[xjamundx]

Yes this got me as well. Any way this can be disabled by default?

[jedwards1211]

Forgive me if this is a dumb question, but why does Flow parse .json files (besides package.json) to begin with?

[vkurchatkin]

@jedwards1211 Flow does this to support requireing .json files

[jedwards1211]

@vkurchatkin I'm not sure I understand why it needs to actually parse them to do that though. For instance it doesn't give me any errors if I try to use the version from package.json as a number:

// @flow

var version: number = require('./package.json').version

I assume it does parse package.jsons for things like "main": "src/index.js" though. But that wouldn't explain why it's parsing other .json files.

[Macil]

Any new feedback on this idea? #869 still gets a lot of user attention, and this is possibly a cleaner and easier to implement solution that solves the issues of most if not all commenters there. (I seem to be the only one ever talking about @flow files existing under node_modules; for modules without Flow types, the only thing that causes errors is malformed JSON files.)

If the solution sounds too over-specific, then maybe a better phrasing of it is "Only report parse errors in @flow files". (The node_modules-specific part could be optional.)

[ericsoco]

For someone attempting to integrate Flow into an existing project as his first experience with Flow, this leaves me with a bad taste...would be great to get some maintainer eyeballs on this.

[pietrofxq]

+1, Flow is parsing a bower.json file that has a comment inside and is throwing errors for me.

[brandly]

i'm running into this with electron-packager:

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ node_modules/electron-packager/test/fixtures/infer-malformed-json/package.json:3:1

Unexpected end of input

     1│ {
     2│   "productName": "InferMalformedJSON",
     3│

i don't understand why it still fails even when i explicitly [ignore] that file in my .flowconfig

[gordysc]

@brandly if you haven't solved this yet, you can accomplish this by doing something like this (notice the .* prefix):

[ignore]
.*/node_modules/electron-packager/test/fixtures/infer-malformed-json/package.json

[bsmith-cycorp]

Comically, Cypress has

node_modules/cypress/dist/Cypress.app/Contents/Resources/app/packages/server/node_modules/jsonlint/test/fails/

which is a directory full of nothing but JSON syntax errors... all of which Flow so-helpfully complains about.

[chrisbobbe]

some of them are opinionated about putting their tests on npm

browserify/resolve#262 (comment) is an example of this:

@TrySound Tests are published in all my packages because npm explore foo && npm install && npm test should always work.

I think that might be reasonable? Here's a doc for npm test.