Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flow is trying to parse JSON files as JS #1420

Closed
grncdr opened this issue Feb 18, 2016 · 16 comments
Closed

Flow is trying to parse JSON files as JS #1420

grncdr opened this issue Feb 18, 2016 · 16 comments
Assignees
Labels

Comments

@grncdr
Copy link
Contributor

grncdr commented Feb 18, 2016

A small repo to demonstrate: https://github.com/grncdr/flowtest, running flow status produces the following output:

src/wat.json:1
  1: []
     ^ Unexpected token [


Found 1 error

This happens on the current master: 0792714, I don't know when it was introduced. It currently breaks flow for most any project using a non-trivial amount of npm packages, as these often have static JSON files for test fixtures and whatnot that flow will now find and break on.

Other than that, the intersection & union type fixes in master are awesome! Keep up the great work on Flow! ❤️ ❤️

@grncdr
Copy link
Contributor Author

grncdr commented Feb 21, 2016

Still happening with the 0.22 release.

@arypurnomoz
Copy link

0.21 was okay i only got this problem on 0.22

@jamiter
Copy link

jamiter commented Feb 23, 2016

A temporary solution is to ignore the JSON files:

[ignore]
.*/node_modules/.*\.json$

@grncdr
Copy link
Contributor Author

grncdr commented Feb 23, 2016

@jamiter unfortunately that seems to cause flow to ignore package.json files, which then causes any modules that make use of the "main" field in package.json to be unresolvable.

@grncdr
Copy link
Contributor Author

grncdr commented Feb 23, 2016

for reference this was the regex I ended up with in my [ignore]:

.*\(lib\|test\|builtin-modules\|binary-extensions\|faker\|spdx\).*\.json

(as far as I can tell Ocaml regexps don't support negative look-behind)

@unscriptable
Copy link

Yep. 0.21 worked fine. 0.22 is b0rked. The only work-around I can find is what @grncdr suggests: explicitly ignoring all of the json files in node_modules/ except package.json files.

@jamiter
Copy link

jamiter commented Feb 23, 2016

@grncdr, thank you! That explained those unresolvable errors! (Duh) Finally have it all working again. This bug of course still needs to be fixed.

I shortened it for myself:

.*/node_modules/.*[^e]\.json$
.*/node_modules/.*\(lib\|test\).*\.json$

This found all the json files I was having issues with. The first line prevents package.json files to be ignored.

@jeffmo
Copy link
Contributor

jeffmo commented Feb 23, 2016

FYI: I just landed #1436 -- which lets Flow parse JSON files that have toplevel array structures (rather than just toplevel object structures).

Based on the error in the OP here, I think that should fix the error that caused that issue!

@jeffmo jeffmo closed this as completed Feb 23, 2016
@mroch
Copy link
Contributor

mroch commented Feb 23, 2016

so we're now compliant with RFC 4627 (the original JSON RFC), thanks!

but we're not with RFC 7159. the latter allows false / null / true / object / array / number / string.

we don't support false, true, null, number or string yet but I think we need to. gonna reopen to track for myself.

@mroch mroch reopened this Feb 23, 2016
@jedwards1211
Copy link
Contributor

@jamiter unfortunately none of these absurd workarounds work if you use a module with a name like owasp-password-strength-test

@jamiter
Copy link

jamiter commented Feb 24, 2016

@jedwards1211, it indeed is not an ideal solution, but they are working on a fix. You can tweak the regex a little to make it work for you. This should solve your issue:

.*/node_modules/.*/\(lib\|test\).*\.json$

The extra slash will make sure that lib or test are directory names within the package.

@vincemtnz
Copy link

With #1436, it will still fail when a module imports an invalid JSON file (e.g. for a test).

mcalthrop pushed a commit to mcalthrop/opencredo-react-boilerplate-1 that referenced this issue Mar 1, 2016
Note: had to revet `flow-bin` to v0.21.0, because of a current bug where JSON files generate Flow errors:
facebook/flow#1420
Once this bug has been fixed, we can bump the version beyond 0.22.0.
@samwgoldman
Copy link
Member

@eyko Yeah, in that case your best bet is to add a pattern targeting those files to the [ignore] section of your .flowconfig. We're looking into fixes that would require zero user intervention as well.

@samwgoldman
Copy link
Member

You might also consider opening an issue with those module authors, informing them that they are including tests in their npm package, which they probably didn't intend to do.

@gajus
Copy link

gajus commented Nov 8, 2016

I am getting a related error for malformed JSON, e.g.

➜  isomorphic-webpack git:(master) ✗ flow version
Flow, a static type checker for JavaScript, version 0.34.0
➜  isomorphic-webpack git:(master) ✗ flow
node_modules/conventional-changelog-core/test/fixtures/_malformation.json:1
  1:
     ^ Unexpected end of input


Found 1 error

In this case, node_modules/conventional-changelog-core/test/fixtures/_malformation.json is just an empty file.

@Macil
Copy link
Contributor

Macil commented Nov 8, 2016

@gajus An empty string is invalid JSON, so that error message is accurate and not completely relevant to this issue. Though there is the separate question of whether it's useful for Flow to report that specific case. You might be interested in this issue: #2364

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests