Open
Bug 1488262
Opened 6 years ago
Updated 2 years ago
Improve the error thrown when a user forgets type="module"
Categories
(Core :: JavaScript Engine, enhancement, P3)
Tracking
()
NEW
People
(Reporter: michiel, Unassigned)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file)
(deleted),
patch
|
dpino
:
feedback?
|
Details | Diff | Splinter Review |
Writing a piece of modern JS using module imports may cause a user to run into a situation that has a very simple solution, but they would never be able to tell from the error that we throw.
Using <script src="somefile.js"></script> instead of <script src="somefile.js" type="module"></script> currently generates the opaque error "SyntaxError: import declarations may only appear at top level of a module" where it should really be generating "SyntaxError: import declarations may only be used by scripts that specify 'module' as type".
The information's already there, so rather than outright throwing a SyntaxError("import declarations may only appear at top level of a module") we can throw the equivalent of SyntaxError(script.type.equals("module")?"import declarations may only appear at top level of a module":"import declarations may only be used by scripts that specify 'module' as type").
And the same goes for exports. Rather than throwing "SyntaxError: import declarations may only appear at top level of a module" files with `export ...` (but no import to trigger the throw) should generated the same kind of informative error, probably just replacing import with export, to give "SyntaxError: export declarations may only be used by scripts that specify 'module' as type".
Comment 2•6 years ago
|
||
Jon, can you triage this? I'm not sure what the right component is. Thanks.
Flags: needinfo?(jcoppeard)
Comment 3•6 years ago
|
||
The error message is part of the JS component.
Component: DOM: Core & HTML → JavaScript Engine
Flags: needinfo?(jcoppeard)
Comment 4•6 years ago
|
||
I don't think the error message would even be correct, when considering eval. I would suggest adding a MDN page for the error instead.
Updated•6 years ago
|
Blocks: harmony:modules
Comment 5•6 years ago
|
||
I don't know if in the end the issue should be fixed by improving the documentation or by handling this condition in code. In case the latter makes sense, I'm attaching a patch.
What the patch does is to check whether the scope is GlobalContext before doing the top level and module check.
Attachment #9006522 -
Flags: feedback?
In terms of a resolution, it's probably worth not picking one or the other but doing both: patch the code for what we know we can catch, without crazy codepaths to catch all the edge cases (new Function, eval, document.createElement('script').textConten, etc.) and then also update the docs so that searching for the error text online leads people to docs that have some variation of "The most common occurence of this error is due to not using 'module' as script type" above the fold.
Comment 7•6 years ago
|
||
we might be able to add note (JSErrorNotes) to the error if it appears at the top level of non-module script.
but would be better moving the detailed solutions to MDN.
Priority: -- → P3
Comment 8•6 years ago
|
||
(In reply to Tooru Fujisawa [:arai] from comment #7)
> we might be able to add note (JSErrorNotes) to the error if it appears at
> the top level of non-module script.
to be clear, I think we should say "top level of a module" in the error message in either case.
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•