Closed Bug 1347709 Opened 6 years ago Closed 6 years ago

Unexpected require when using eslint-plugin-mozilla@0.2.29

Categories

(Developer Infrastructure :: Lint and Formatting, defect)

Product:
Developer Infrastructure
Component:
Lint and Formatting
Version:
3 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: pdehaan, Assigned: standard8)

References

Details

Attachments

(3 files)

Bug 1347709 - Add a .npmignore file to stop publishing unnecessary files for eslint-plugin-mozilla.
59 bytes, text/x-review-board-request
mossop
: review+
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.
59 bytes, text/x-review-board-request
mossop
: review+
Details
Bug 1347709 - Allow modules.json to be loaded from a local version for out-of-tree uses of eslint-plugin-mozilla.
59 bytes, text/x-review-board-request
mossop
: review+
Details 
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).
Blocks: 1347645
Assignee: nobody → standard8
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/
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 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 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 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+
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
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
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?
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.
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).
Actually, for some reason 0.2.39 wasn't published right. 0.2.40 works.
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.