Unexpected require when using eslint-plugin-mozilla@0.2.29

RESOLVED FIXED in Firefox 55

Status

Testing
Lint
RESOLVED FIXED
a year ago
10 months ago

People

(Reporter: pdehaan, Assigned: standard8)

Tracking

Version 3
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

a year ago
Ref: https://github.com/mozilla-services/pageshot/issues/2365#issuecomment-286882614

In Page Shot, we're trying to use **eslint-plugin-mozilla@0.2.29** and are getting errors because eslint-plugin-mozilla is trying to require a non-existent file (in eslint-plugin-mozilla/lib/environments/places-overlay.js):


```
var root = helpers.getRootDir(module.filename);

var tmp = path.join(root, "tools", "lint", "eslint", "modules.json");

console.log(tmp);
var modules = require(tmp);
```

Locally, this logs the following value: 

/Users/pdehaan/dev/github/mozilla-services/pageshot/tools/lint/eslint/modules.json


I'm guessing that's why this is crashing hard, because we don't have a "tools/lint/eslint/modules.json" file in tbe Page Shot repo tree (but it looks like it _is_ a path in the m-c repo: http://searchfox.org/mozilla-central/source/tools/lint/eslint/modules.json).
(Assignee)

Updated

a year ago
Blocks: 1347645
(Assignee)

Updated

11 months ago
Assignee: nobody → standard8

Comment 2

11 months ago
I also ran into this for activity-stream, and a quick workaround at least for local development is to symlink to tools from the top level of your repository:

https://github.com/mozilla/activity-stream/pull/2304

ln -s ../mozilla-central/tools/
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 6

11 months ago
I've tried the patches against the one-off-system-addon & screenshots repo, and the code seems to work & rules pass.

Once this lands I'll get a new npm version published and start work on adding it to those repos properly.

Comment 7

11 months ago
mozreview-review
Comment on attachment 8858136 [details]
Bug 1347709 - Add a .npmignore file to stop publishing unnecessary files for eslint-plugin-mozilla.

https://reviewboard.mozilla.org/r/130096/#review132992
Attachment #8858136 - Flags: review?(dtownsend) → review+

Comment 8

11 months ago
mozreview-review
Comment on attachment 8858137 [details]
Bug 1347709 - Add a prepublish script to save the current globals for the published version of eslint-plugin-mozilla, and use that when not in mozilla-central.

https://reviewboard.mozilla.org/r/130098/#review132998
Attachment #8858137 - Flags: review?(dtownsend) → review+

Comment 9

11 months ago
mozreview-review
Comment on attachment 8858138 [details]
Bug 1347709 - Allow modules.json to be loaded from a local version for out-of-tree uses of eslint-plugin-mozilla.

https://reviewboard.mozilla.org/r/130100/#review133008

::: tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js:47
(Diff revision 1)
>  
>  module.exports = {
> +  get modulesGlobalData() {
> +    if (!gModules) {
> +      if (this.isMozillaCentralBased()) {
> +        console.log("m-c based");

Remove the logging please
Attachment #8858138 - Flags: review?(dtownsend) → review+
Comment hidden (mozreview-request)

Comment 11

11 months ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/af51323bf014
Add a .npmignore file to stop publishing unnecessary files for eslint-plugin-mozilla. r=mossop
https://hg.mozilla.org/integration/autoland/rev/2153975f1124
Add a prepublish script to save the current globals for the published version of eslint-plugin-mozilla, and use that when not in mozilla-central. r=mossop
https://hg.mozilla.org/integration/autoland/rev/c873d3edeae6
Allow modules.json to be loaded from a local version for out-of-tree uses of eslint-plugin-mozilla. r=mossop

Comment 12

11 months ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/mozilla-central/rev/ce69b6e1773e
and Bug 1356569 - Bump eslint plugin version to resolve conflict. r=instruction-by-florian a=bustage-fix
https://hg.mozilla.org/mozilla-central/rev/af51323bf014
https://hg.mozilla.org/mozilla-central/rev/2153975f1124
https://hg.mozilla.org/mozilla-central/rev/c873d3edeae6
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Comment 14

11 months ago
What's the expected behavior with these changes? Repositories must add a `modules.json` in its root directory with keys of module filenames and values of string array of exported symbols?

Comment 15

11 months ago
I'm running into some issues after `npm install mozilla-central/tools/lint/eslint/eslint-plugin-mozilla`

Looks like it's trying to read a "./modules.json" relative to eslint-plugin-mozilla/lib/helpers.js:
    at Object.get modulesGlobalData [as modulesGlobalData] (activity-stream/node_modules/eslint-plugin-mozilla/lib/helpers.js:49:20)

If I symlink to a modules.json that I put at the top level, it then

Error: Cannot find module './environments/saved-globals.json'
    at Object.getSavedEnvironmentItems (activity-stream/node_modules/eslint-plugin-mozilla/lib/helpers.js:593:12)

Maybe I'll just wait for Standard8 to fix another repository, and I'll just copy. :)
You need to wait for Standard8 to publish the module to npm then install that.
(Assignee)

Comment 17

10 months ago
Ed: There's a prepublish script that gets run, however you can run that manually before importing the module locally.

I also have just published it to npm, so you can pick up the new version (0.2.39) there (and there's a minor bug in the prepublish script that I'll publish a fix for tomorrow).
(Assignee)

Comment 18

10 months ago
Actually, for some reason 0.2.39 wasn't published right. 0.2.40 works.
You need to log in before you can comment on or make changes to this bug.