Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS parse error (due to bleeding-edge feature use) in <script type="module"> causes spurious error `'import' and 'export' may appear only with 'sourceType: module' #286

Open
jameshfisher opened this issue Nov 11, 2020 · 5 comments
Labels
bug Something isn't working

Comments

@jameshfisher
Copy link

Thanks for this plugin! It has caught quite a few genuine errors, but here's one that I think is spurious.

The essence of the page is:

<script type="module">
  import { h, Component, render } from 'https://unpkg.com/preact?module';
  // ...
</script>

This gives me the error:

12:05:11 PM:   ✖ FAIL Failed loading inline JavaScript in foo/bar/baz.html
12:05:11 PM:   | operator: error
12:05:11 PM:   |   actual: inline JavaScript in foo/bar/baz.html: Parse error in inline JavaScript in foo/bar/baz.html

I'm guessing your JS parser needs to know about the type of the <script>.

@Munter
Copy link
Owner

Munter commented Nov 11, 2020

I think you might be right on the parser error.

@papandreou this is all pretty long ago in my mind. Could you remind me what the state of JS parsing capabilities of modern modules is in assetgraph?

@Munter Munter added the bug Something isn't working label Nov 11, 2020
@papandreou
Copy link

I cannot reproduce that? Munter/hyperlink@7ce64a9

Which version of hyperlink are you using? What does npm ls assetgraph output? (I tested with 6.2.2)

@papandreou
Copy link

I think the most likely cause is that there some other problem or syntax error in the parts of the inline script that you omitted (// ...), which means that we end up in the fallback to parsing it as sourceType: 'script' here: https://github.com/assetgraph/assetgraph/blob/19024af0e6a16bf9c5a8a86e6d40a16e405fecd1/lib/assets/JavaScript.js#L232

If that's the case, I do agree that the error message could be better. We could maybe detect errors referring to features that are only supported for modules in the fallback, and then emit the original error in that case.

@jameshfisher
Copy link
Author

I think you're right! I came up with this minimal test case:

<!doctype html>
<html>
  <body>
    <script type="module">
      import { Component } from 'https://unpkg.com/preact?module';
      class Foo { state = {}; }
    </script>
  </body>
</html>

The state = {}; is sort of an error on my part. What I meant to write was class Foo { constructor() { this.state = {}; } }. It seems what I unwittingly declared was a field declaration, which works much the same way, so went unnoticed. Public field declarations are an experimental feature, but apparently have support in Chrome. I'm guessing hyperlink's parser does not.

I suppose the behavior I would like is, if hyperlink can't parse the module JS, just show me the parse error, rather than catching the error and trying again as script.

Either way, thanks for the help!

@jameshfisher jameshfisher changed the title Spurious error 'import' and 'export' may appear only with 'sourceType: module' in <script type="module"> JS parse error (due to bleeding-edge feature use) in <script type="module"> causes spurious error `'import' and 'export' may appear only with 'sourceType: module' Nov 11, 2020
papandreou added a commit to assetgraph/assetgraph that referenced this issue Nov 11, 2020
@papandreou
Copy link

Fixed in assetgraph 6.3.0 by reporting the original error (via assetgraph/assetgraph@918f2f9) which is in range. Try updating 🤗

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants