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

Problems in IntelliJ when node_modules folder is not on top level #8

Closed
DJCordhose opened this issue Sep 26, 2016 · 28 comments
Closed

Comments

@DJCordhose
Copy link

DJCordhose commented Sep 26, 2016

I tried out the plugin and it works great as a webpack loader, but I can not make it run in Webstorm / IntelliJ. The problem is that it does not seem to find the plugin as the directory it looks in is not right (should be ...src/main/js/...):

Error: Cannot find module '/Users/olli/Development/floreysoft/floreysoft-status/node_modules/eslint-plugin-flowtype-errors/dist/collect.js'
at Function.Module._resolveFilename (module.js:455:15)
at Function.Module._load (module.js:403:25)
at Module.runMain (module.js:590:10)
at run (bootstrap_node.js:394:7)
at startup (bootstrap_node.js:149:9)
at bootstrap_node.js:509:3

at checkExecSyncError (child_process.js:483:13)
at execSync (child_process.js:523:13)
at showErrors (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint-plugin-flowtype-errors/index.js:36:25)
at /Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint/lib/eslint.js:824:25
at Array.forEach (native)
at EventEmitter.module.exports.api.verify (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint/lib/eslint.js:796:16)
at processText (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint/lib/cli-engine.js:262:31)
at CLIEngine.executeOnText (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint/lib/cli-engine.js:751:26)
at Object.execute (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint/lib/cli.js:181:36)
at /Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint/bin/eslint.js:57:36

node node_modules/eslint-plugin-flowtype-errors/dist/collect.js

Looking at the code:

const collected = execSync(
  `node ${path.normalize('./node_modules/eslint-plugin-flowtype-errors/dist/collect.js')}`
);

it becomes clear why there is this error: my node_modules are simply somewhere else.

Any ideas? Add a configuration path pointing to the location of the node_modules?

This might simply be because of my messed up set up, but without the plugin it works well and it can also load other plugins without a problem.

@amilajack
Copy link
Owner

I'm assuming that you're running ESLint locally. Is that right?
Also what is the output of cat '/Users/olli/Development/floreysoft/floreysoft-status/node_modules/eslint-plugin-flowtype-errors/dist/collect.js. Does anything actually exist there?

@DJCordhose
Copy link
Author

As I said, there is nothing there as my node_modules are not installed at root level. The problem is the path to the node_module that is set as fixed in the code (again, as described above).

@amilajack
Copy link
Owner

Add a configuration path pointing to the location of the node_modules

This is a solution but ideally, it should work with no configuration. I think a possible solution is somehow finding the absolute path of the directory of theeslint installation.

Maybe something like this:

path.join(absolutePathToEslint, '../eslint-plugin-flowtype-errors/dist/collect.js')

Thoughts?

@DJCordhose
Copy link
Author

Would be the best solution.

@DJCordhose
Copy link
Author

I did a small hack to fix the path, but there are more errors and I am not sure what to do there.

Looks like firstMessage does not have a loc.

/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint-plugin-flowtype-errors/dist/collect.js:112
start: firstMessage.loc.start.line,

TypeError: Cannot read property 'start' of undefined
at /Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint-plugin-flowtype-errors/dist/collect.js:112:30
at Array.map (native)
at executeFlow (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint-plugin-flowtype-errors/dist/collect.js:86:39)
at Flow (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint-plugin-flowtype-errors/dist/collect.js:124:10)
at Object. (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint-plugin-flowtype-errors/dist/collect.js:127:47)
at Module._compile (module.js:556:32)
at Object.Module._extensions..js (module.js:565:10)
at Module.load (module.js:473:32)
at tryModuleLoad (module.js:432:12)
at Function.Module._load (module.js:424:3)

@amilajack
Copy link
Owner

amilajack commented Sep 26, 2016

What was the quick hack that you did? Did it include hardcoding the correct path?

@wukkuan
Copy link

wukkuan commented Sep 28, 2016

I'm having the same issue, though with flycheck in emacs.

I was able to get past the original error (Error: Cannot find module) by configuring flycheck to use the root of the git repo as the working directory. Then I started getting the same error as @DJCordhose.

Doing some more investigation, process.cwd() at that point in the code says that I'm in the directory above my git repo. It looks like it's happening in eslint itself, because when I check process.cwd() in the eslint bin, it's in the correct directory.

@amilajack
Copy link
Owner

amilajack commented Sep 28, 2016

What version of the module are you using? I made a few updates recently.

@wukkuan
Copy link

wukkuan commented Sep 28, 2016

Ah, bumping to the latest (1.2.3) seems to have resolved the problems! Thank you!

FWIW I was on 1.2.0 before.

@amilajack
Copy link
Owner

@wukkuan Thanks for the feedback!

@DJCordhose Try updating the package. That should do the trick.

@DJCordhose
Copy link
Author

DJCordhose commented Sep 28, 2016

Not finding collect.js has been fixed in 1.2.3, but I still see the next error:

      start: firstMessage.loc.start.line,
                             ^

TypeError: Cannot read property 'start' of undefined
    at /Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint-plugin-flowtype-errors/dist/collect.js:105:30
    at Array.map (native)
    at executeFlow (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint-plugin-flowtype-errors/dist/collect.js:90:39)
    at Flow (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint-plugin-flowtype-errors/dist/collect.js:117:10)
    at Object.<anonymous> (/Users/olli/Development/floreysoft/floreysoft-status/src/main/js/node_modules/eslint-plugin-flowtype-errors/dist/collect.js:120:47)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)```

@amilajack
Copy link
Owner

amilajack commented Sep 28, 2016

Is your .flowconfig located in the root of your project?

@DJCordhose
Copy link
Author

Hey, good catch. It is not. When I move it to the root everything works just fine :)

Do you think it is possible to not require it at root level? Because my project is a mix of Java and JavaScript and it would be weird to have it above my Java folder.

Anyway, errors are displayed inline in IntelliJ, added a screen shot for our entertainment :)

screen shot 2016-09-28 at 16 30 50

@amilajack
Copy link
Owner

amilajack commented Sep 28, 2016

@DJCordhose I can work on adding that feature today. I'm thinking allowing that to be configured in the .eslintrc. If the project can't find the .flowconfig, it should throw an error at installation time and runtime.

@DJCordhose
Copy link
Author

+1

@amilajack
Copy link
Owner

amilajack commented Oct 1, 2016

@DJCordhose In eclipse, do you have to save a file for the Flow errors to update in the IDE? For example, if you have this:

const example: string = 1 // <-- line 1

and add a new space:

// line 1
const example: string = 1 // <-- line 2

Do you have to have to save the file for the flow errors to update and report on the new line?

@DJCordhose
Copy link
Author

DJCordhose commented Oct 1, 2016

I use IntelliJ, not Eclipse, but in IntelliJ you get the errors without saving explicitly. Very nice 👍

@pelotom
Copy link

pelotom commented Oct 1, 2016

In Atom with the linter-eslint package the warnings/errors also appear as you type, without having to save.

@amilajack
Copy link
Owner

I see. So it is as @DJCordhose described it?

you get the errors without saving explicitly

@pelotom
Copy link

pelotom commented Oct 1, 2016

Yes.

@amilajack
Copy link
Owner

flow_on_the_fly_save

Here's a short demo showing how this doesn't work with my configuration. I only save the file once in the demo. Notice how removing the :string annotation doesn't lint on the fly.

@pelotom @DJCordhose so just to confirm, you guys aren't experiencing anything like this?

@pelotom
Copy link

pelotom commented Oct 1, 2016

@amilajack sorry, actually I am seeing wonky behavior specifically with errors from flowtype-errors... all other ESLint warnings/errors seem to work on the fly, but flowtype-errors seem to only be triggered on save :(

@amilajack
Copy link
Owner

amilajack commented Oct 1, 2016

I'm using atom, linter-eslint and linter as well so I had a feeling that you would experience the same issues as well. This is something that really puzzles me. It could be that these lines are taking too long the execute and ESLint/atom-eslint/atom-linter might not wait for it to finish because it takes too long. For me, I have to save the file twice for it to take effect 😢 But Flow does run on every single rule check so I don't know why this is the behavior.

@amilajack
Copy link
Owner

@pelotom @DJCordhose I've fixed the issues. Now it should work on the fly without having to save! I would really appreciate if you could test this out before I publish the package.

npm i -D eslint-plugin-flowtype-errors@1.2.6-alpha.1

@DJCordhose
Copy link
Author

In IntelliJ it worked even before and it still works with 1.2.6 :)

@amilajack
Copy link
Owner

Yes! Nothing broke!

@DJCordhose
Copy link
Author

Yes, not in IntelliJ :)

@amilajack
Copy link
Owner

Also what does the error

inconsistent use of library definitions:  null This type is incompatible with string

mean? Does it meant the library definitions can't be found? On what line numbers should these kinds of errors be reported on?

This output from flowtype.org/try gives no specific line on which the errors occurs:

. inconsistent use of library definitions
/static/v0.33.0/flowlib/node.js:1489:   env : { [key: string] : ?string };
                                                                 ^ null. This type is incompatible with
/static/v0.33.0/flowlib/node.js:1285:     hostname?: string;
                                                     ^ string
. inconsistent use of library definitions
/static/v0.33.0/flowlib/node.js:1489:   env : { [key: string] : ?string };
                                                                 ^ null. This type is incompatible with
/static/v0.33.0/flowlib/node.js:1287:     host?: string;
                                                 ^ string
234: export function handleProviderError(error) {
                                         ^ parameter `error`. Missing annotation

The last error reports a line but the 'inconsistent use of library definitions' errors don't :(

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

No branches or pull requests

4 participants