1330688 - Using the wrong name when importing an ES6 module breaks the script and doesn't log any error

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Dave Townsend [:mossop]]

I had a module that looked like this:

    alert('foo');
    import foo from './store';
    alert('foo');

I had left off the ".js" extension from the module to import. I saw neither of the alerts appear and no error in the webconsole to help me track down the error.

Comment 1

[Jon Coppeard (:jonco)]

Module imports are loaded before the script is executed so you won't see either alert, but we could produce an error to make problems easier to track down.

Comment 2

[Dave Townsend [:mossop]]

Yeah, some message that the module could not be found would have been really useful for tracking down the problem.

Comment 3

[Tom S]

Since bug 247996 the console shows:
> Loading failed for the <script> with source “http://0.0.0.0:8000/store”. test.html:1

We should probably change <script> to module?

Comment 4

[Jon Coppeard (:jonco)]

Testing with: data:text/html,%3Cscript type=module src=http://www.example.com/missing%3E%3C%2Fscript%3E

Comment 5

[Jon Coppeard (:jonco)]

Attachment: bug1330688-module-console-error

Patch to change console messages to refer to either a script or a module.

Comment 6

[Andrea Marchesini [:baku]]

Comment on attachment 8935811 [details] [diff] [review]
bug1330688-module-console-error

Review of attachment 8935811 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/script/ScriptLoadRequest.h
@@ +254,5 @@
>                              const char* aName,
>                              uint32_t aFlags);
>  
> +const char16_t*
> +ScriptKindName(ScriptKind kind);

can you make this a static method in ScriptLoadRequest?

Comment 7

[Andrea Marchesini [:baku]]

Comment on attachment 8935811 [details] [diff] [review]
bug1330688-module-console-error

flod, can you check the dom.properties part of this patch?

Comment 8

[Francesco Lodolo [:flod]]

Comment on attachment 8935811 [details] [diff] [review]
bug1330688-module-console-error

Review of attachment 8935811 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, all the string IDs need to change, since you changed the text.
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

Also:
* Use %1$S, %2$S, etc. when there's more than one placeholder
* If there's a placeholder, comment should explain what they are (what's going to replace them). You changed all the strings, and now they have comments that are not relevant anymore (there's no <script> in the text)

Comment 9

[Jon Coppeard (:jonco)]

(In reply to Francesco Lodolo [:flod] from comment #8)
Thanks for that, I'll redo the patch so that this can be localised correctly.

Comment 10

[Jon Coppeard (:jonco)]

Attachment: bug1330688-module-console-error v2

I updated the patch to make separate messages for module errors so these can be localised correctly.

Comment 11

[Jon Coppeard (:jonco)]

Comment on attachment 8936488 [details] [diff] [review]
bug1330688-module-console-error v2

Just wanted to check that this is OK now.

Comment 12

[Francesco Lodolo [:flod]]

Comment on attachment 8936488 [details] [diff] [review]
bug1330688-module-console-error v2

Review of attachment 8936488 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/locales/en-US/chrome/dom/dom.properties
@@ +343,3 @@
>  # LOCALIZATION NOTE: Do not translate "<script>".
>  ScriptSourceMalformed=<script> source URI is malformed: â%Sâ.
> +ModuleSourceMalformed=module source URI is malformed: â%Sâ.

Is there a reason for this string and ModuleSourceNotAllowed to start lowercase?

Comment 13

[Jon Coppeard (:jonco)]

(In reply to Francesco Lodolo [:flod] from comment #12)
No reason, I'll fix that.

Comment 14

[Jon Coppeard (:jonco)]

Attachment: bug1330688-module-console-error v3

Comment 15

[Francesco Lodolo [:flod]]

Comment on attachment 8937425 [details] [diff] [review]
bug1330688-module-console-error v3

Review of attachment 8937425 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, it looks good. 

Nice how hg diff still screws up in 2017 with UTF-8 characters appearing in the @@ line…

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e6356ea82b2
Report a better error in the console when a module load fails r=baku r=flod

Comment 17

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/8e6356ea82b2