[eslint] importScripts global handling should fallback to the file name if there is no entry in modules.json

RESOLVED FIXED

Status

enhancement
P2
normal
RESOLVED FIXED
7 months ago
7 months ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

(Blocks 1 bug)

Trunk
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

7 months ago
convertWorkerExpressionToGlobals doesn't fall back to using the filename as the global name if it doesn't find an entry in modules.json.

We should do this to make it easier for new modules and avoid entries in modules.json if we don't need them.

convertCallExpressionToGlobals already has this for other import types.
Assignee

Comment 1

7 months ago
Note: as a follow-up we should also see if we can remove most of the single mappings from modules.json (i.e. "foo.js" -> "foo").
Assignee

Updated

7 months ago
Priority: -- → P2
Assignee

Comment 2

7 months ago
We already have a fallback for items that aren't in a worker scope, we should have the same for ones that are. This means we don't need single maps (foo.js -> foo) in modules.json, and also we can identify more as explicit variables, so that no-unused-vars can detect them.
Assignee

Updated

7 months ago
Assignee: nobody → standard8

Comment 3

7 months ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f69dc19ed7c
Handling of importScripts by ESLint should fallback to the file name if there is no entry in modules.json. r=mossop
Backed out for devtools failures on browser_scratchpad_pprint.js

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&fromchange=6f69dc19ed7cd1277fda93cd8e3129528f23bbc4&tochange=39b671c8480bc70b9f82492e06bde573d3489fc1&searchStr=devtools&group_state=expanded&selectedJob=216207628

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=216207628&repo=autoland&lineNumber=11372

Backout link: https://hg.mozilla.org/integration/autoland/rev/39b671c8480bc70b9f82492e06bde573d3489fc1

[task 2018-12-10T16:53:05.119Z] 16:53:05     INFO - TEST-UNEXPECTED-FAIL | devtools/client/scratchpad/test/browser_scratchpad_pprint-02.js | Test timed out - 
[task 2018-12-10T16:53:05.119Z] 16:53:05     INFO - GECKO(3147) | MEMORY STAT | vsize 20974246MB | residentFast 1415MB
[task 2018-12-10T16:53:05.119Z] 16:53:05     INFO - TEST-OK | devtools/client/scratchpad/test/browser_scratchpad_pprint-02.js | took 90272ms
[task 2018-12-10T16:53:05.119Z] 16:53:05     INFO - checking window state
[task 2018-12-10T16:53:05.120Z] 16:53:05     INFO - TEST-START | devtools/client/scratchpad/test/browser_scratchpad_pprint.js
[task 2018-12-10T16:53:06.291Z] 16:53:06     INFO - GECKO(3147) | Error: ReferenceError: require is not defined @ resource://devtools/shared/pretty-fast/pretty-fast.js:20
[task 2018-12-10T16:54:35.190Z] 16:54:35     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-12-10T16:54:35.191Z] 16:54:35     INFO - Buffered messages logged at 16:53:06
[task 2018-12-10T16:54:35.191Z] 16:54:35     INFO - Console message: [JavaScript Warning: "XUL box for toolbar element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "chrome://devtools/content/sourceeditor/codemirror/codemirror.bundle.js" line: 1}]
[task 2018-12-10T16:54:35.191Z] 16:54:35     INFO - Console message: [JavaScript Warning: "XUL box for toolbar element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "chrome://devtools/content/scratchpad/index.xul" line: 0}]
[task 2018-12-10T16:54:35.192Z] 16:54:35     INFO - Console message: [JavaScript Warning: "XUL box for toolbar element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "chrome://devtools/content/sourceeditor/codemirror/codemirror.bundle.js" line: 1}]
[task 2018-12-10T16:54:35.192Z] 16:54:35     INFO - Console message: [JavaScript Error: "ReferenceError: require is not defined" {file: "resource://devtools/shared/pretty-fast/pretty-fast.js" line: 20}]
[task 2018-12-10T16:54:35.192Z] 16:54:35     INFO - Buffered messages finished
[task 2018-12-10T16:54:35.193Z] 16:54:35     INFO - TEST-UNEXPECTED-FAIL | devtools/client/scratchpad/test/browser_scratchpad_pprint.js | Test timed out - 
[task 2018-12-10T16:54:35.336Z] 16:54:35     INFO - GECKO(3147) | MEMORY STAT | vsize 20974283MB | residentFast 1351MB
[task 2018-12-10T16:54:35.336Z] 16:54:35     INFO - TEST-OK | devtools/client/scratchpad/test/browser_scratchpad_pprint.js | took 90220ms
[task 2018-12-10T16:54:35.413Z] 16:54:35     INFO - checking window state
[task 2018-12-10T16:54:35.451Z] 16:54:35     INFO - TEST-START | devtools/client/scratchpad/test/browser_scratchpad_pprint_error_goto_line.js
[task 2018-12-10T16:54:36.679Z] 16:54:36     INFO - GECKO(3147) | Error: ReferenceError: require is not defined @ resource://devtools/shared/pretty-fast/pretty-fast.js:20
[task 2018-12-10T16:56:05.519Z] 16:56:05     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-12-10T16:56:05.519Z] 16:56:05     INFO - Buffered messages logged at 16:54:36
[task 2018-12-10T16:56:05.520Z] 16:56:05     INFO - Console message: [JavaScript Warning: "XUL box for toolbar element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "chrome://devtools/content/sourceeditor/codemirror/codemirror.bundle.js" line: 1}]
[task 2018-12-10T16:56:05.520Z] 16:56:05     INFO - Console message: [JavaScript Warning: "XUL box for toolbar element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "chrome://devtools/content/scratchpad/index.xul" line: 0}]
[task 2018-12-10T16:56:05.521Z] 16:56:05     INFO - Console message: [JavaScript Warning: "XUL box for toolbar element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "chrome://devtools/content/sourceeditor/codemirror/codemirror.bundle.js" line: 1}]
[task 2018-12-10T16:56:05.521Z] 16:56:05     INFO - Console message: [JavaScript Error: "ReferenceError: require is not defined" {file: "resource://devtools/shared/pretty-fast/pretty-fast.js" line: 20}]
[task 2018-12-10T16:56:05.522Z] 16:56:05     INFO - Buffered messages finished
[task 2018-12-10T16:56:05.522Z] 16:56:05     INFO - TEST-UNEXPECTED-FAIL | devtools/client/scratchpad/test/browser_scratchpad_pprint_error_goto_line.js | Test timed out - 
[task 2018-12-10T16:56:05.659Z] 16:56:05     INFO - GECKO(3147) | MEMORY STAT | vsize 20974273MB | residentFast 1343MB
Flags: needinfo?(standard8)
Assignee

Comment 5

7 months ago
The issue is that pretty-fast.js requires the items that I removed, and falls back to require if not found. Unfortunately in the worker scope, require is not found.

So we should just mark the symbols as exported to avoid no-unused-vars issues.
Flags: needinfo?(standard8)
Assignee

Comment 6

7 months ago
I couldn't quite mark them as exported... something isn't quite working right, so for now I've filed bug 1513171 as a follow-up.
Blocks: 1513171
Assignee

Comment 7

7 months ago
It turns out pretty printing in the scratchpad has just been removed (bug 1512434) so that file/tests are no more. I'll still leave bug 1513171 open, as I think it is still a potential issue.

Comment 8

7 months ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c6712f1b0270
Handling of importScripts by ESLint should fallback to the file name if there is no entry in modules.json. r=mossop

Comment 9

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c6712f1b0270
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.