Closed Bug 1222232 Opened 6 years ago Closed 6 years ago

`mach eslint …` gives "Error: Cannot find module 'eslint/node_modules/escope'"

Categories

(Testing :: General, defect)

defect
Not set
normal

Tracking

(firefox45 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: MattN, Assigned: miker)

References

Details

Attachments

(1 file, 4 obsolete files)

`
$ ./mach eslint browser/components/loop/
 0:00.09 Running /usr/local/bin/eslint in browser/components/loop/
 0:00.09 /usr/local/bin/eslint --ext [.js,.jsm,.jsx] .
module.js:339
    throw err;
    ^

Error: Cannot find module 'eslint/node_modules/escope'
    at Function.Module._resolveFilename (module.js:337:15)
    at Function.Module._load (module.js:287:25)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at Object.<anonymous> (/Users/matthew/mozilla-central/testing/eslint-plugin-mozilla/lib/helpers.js:9:14)
    at Module._compile (module.js:425:26)
    at Object.Module._extensions..js (module.js:432:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Module.require (module.js:366:17)
`
I don't know much about node modules or eslint but I guess this is because https://mxr.mozilla.org/mozilla-central/source/testing/eslint-plugin-mozilla/lib/helpers.js uses escope and espreee but those aren't marked as dependencies in the package.json of the mozilla eslint plugin?
Flags: needinfo?(mratcliffe)
escope and espreee are dependencies of eslint so they should be available:
https://github.com/eslint/eslint/blob/master/package.json#L43-L44

Can I ask how you installed eslint?

Can you try running:
sudo ./mach eslint --setup

If it doesn't fix it then try:
npm remove eslint
npm install eslint -g

And let us know if this fixes your issue?
Assignee: nobody → mratcliffe
Flags: needinfo?(mratcliffe)
I'm on OS X btw.

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #1)
> escope and espreee are dependencies of eslint so they should be available:
> https://github.com/eslint/eslint/blob/master/package.json#L43-L44
> 
> Can I ask how you installed eslint?

`./mach eslint --setup`  (without sudo)

> Can you try running:
> sudo ./mach eslint --setup

Same error.

> If it doesn't fix it then try:
> npm remove eslint
> npm install eslint -g
> 
> And let us know if this fixes your issue?

Nope, same error. (Did you intend for this to be with sudo?)
Yes, you could try that with sudo.

Can I ask how you installed eslint? Because escope and espreee are dependencies of eslint eslint/node_modules/escope and eslint/node_modules/espree should exist.

What is the output from:
ls /usr/local/lib/node_modules

and:
ls /usr/local/lib/node_modules/eslint/node_modules/
<reepush> mikeratcliffe: hi, I tried to setup ESLint and there was a problem installing eslint-plugin-mozilla
<reepush> mikeratcliffe: requiring "eslint/node_modules/escope" dependency failed, I think because of my latest version of npm which flattens modules by default

Awesome feedback... thanks reepush.

Flattened modules would explain some of the issues that we have been having. Looking into this I have discovered that this was done to prevent issues with long paths in Windows.

Previously, your project's required modules would be installed in node_modules. If they themselves required modules, they'd be installed in node_modules/{package}/node_modules. The problem is obvious here, but it was surprisingly hard to work around - solutions involving npm shrinkwrap and module-flattening techniques existed, but it isn't pretty.

In npm3, dependencies are now be installed flat by default. If possible, all dependencies, and their dependencies, and their dependencies will be installed in your project's node_modules folder without nesting. Nesting only occurs when two or more modules have conflicting dependencies.

This means that we need to check for the module in /usr/local/lib/node_modules/* and /usr/local/lib/eslint/node_modules/* (with the path corrected depending on your node module path).
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #9)
> W0t... seems like MozReview can't handle multiple bugs in a single push.

It can handle it but you need to tell it which patches you want reviewed with the -c (one rev) or -r (multiple revs) arguments to `hg push`. See the docs at https://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview/review-requests.html#choosing-what-commits-to-review
Comment on attachment 8690133 [details]
MozReview Request: Bug 1222232 - Help `mach eslint` find espree and escope r?=pbrosset

https://reviewboard.mozilla.org/r/25769/#review23287

Looks nice and simple to me.
Attachment #8690133 - Flags: review?(pbrosset) → review+
Attachment #8690134 - Flags: review?(pbrosset)
Comment on attachment 8690134 [details]
MozReview Request: Bug 1217922 - eslint head.js plugin does not seem to work r?=pbrosset

https://reviewboard.mozilla.org/r/25771/#review23289

::: testing/eslint-plugin-mozilla/lib/rules/import-headjs-globals.js:95
(Diff revision 1)
> -      checkFile([testPath, headjs], context);
> +      checkFile([fullTestPath, fullHeadjsPath]);

You're not passing context anymore here as the 2nd argument. Is this intentional?
The checkFile function's signature wasn't changed in this commit:
`function checkFile(fileArray, context)`
so it will still expect a `context` parameter.
Comment on attachment 8690135 [details]
MozReview Request: Bug 1225289 - Create appropriate .eslintrc for eslint-plugin-mozilla folder r?=pbrosset

https://reviewboard.mozilla.org/r/25773/#review23291

::: testing/eslint-plugin-mozilla/lib/rules/.eslintrc:3
(Diff revision 1)
> +  "extends": "../../../../devtools/.eslintrc",

Wait, what's this eslintrc for?
Is it used by eslint to check the content of the testing/eslint-plugin-mozilla/lib/rules/?
If yes, why are the custom mozilla rules enabled?
And why does it extend from the devtools eslintrc file? 
The code around here is generic enough that any code in the tree should be able to use it, so there really shouldn't devtools mentioned here.
Attachment #8690135 - Flags: review?(pbrosset)
Comment on attachment 8690136 [details]
MozReview Request: Bug 1225289 - Make eslint plugin code conform to .eslintrc r?=pbrosset

https://reviewboard.mozilla.org/r/25775/#review23293
Attachment #8690136 - Flags: review?(pbrosset) → review+
Attachment #8690133 - Attachment is obsolete: true
Attachment #8690134 - Attachment is obsolete: true
Attachment #8690135 - Attachment is obsolete: true
Attachment #8690136 - Attachment is obsolete: true
Bug 1222232 - Help `mach eslint` find espree and escope r+=pbrosset

You have already r+ssed this but I am separating the patches.
Attachment #8690765 - Flags: review?(pbrosset)
Attachment #8690765 - Flags: review?(pbrosset) → review?(mratcliffe)
Comment on attachment 8690765 [details]
MozReview Request: Bug 1222232 - Help `mach eslint` find espree and escope r+=pbrosset

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25939/diff/1-2/
Attachment #8690765 - Flags: review?(mratcliffe) → review+
https://hg.mozilla.org/mozilla-central/rev/5868072bfcb5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.