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

Dynamic import and es format #2285

Closed
camille-hdl opened this issue Jun 19, 2018 · 6 comments
Closed

Dynamic import and es format #2285

camille-hdl opened this issue Jun 19, 2018 · 6 comments

Comments

@camille-hdl
Copy link

@camille-hdl camille-hdl commented Jun 19, 2018

Repo with example but not necessary to understand my problem, I think: https://github.com/camille-hdl/rollup-starter-code-splitting

Hello

When using system output format, dynamic import("./file.js") are transpiled to module.import("./file.js") (example), as expected.

When using es format, import("./file.js") is not transpiled. This is also expected, I assume ?

The problem comes with the <script type="module"> tag, which will be interpreted without problem by chrome >=63 (which supports both static and dynamic import), will be ignored by Firefox <60 (and use my nomodule systemjs fallback), but will cause an error in, say, Firefox >=60 (which only supports static import).

Is there a way to compile to es format and transpile dynamic import() syntax ? I have @babel/plugin-syntax-dynamic-import in my .babelrc, and I have tried @babel/preset-stage-3 but this doesn't change anything in the output or in babel debug info.

This is maybe a babel issue, but since rollup is deciding which module format to use, I'm guessing my question belongs here.

@guybedford

This comment has been minimized.

Copy link
Contributor

@guybedford guybedford commented Jun 19, 2018

@camille-hdl I was against nomodule landing exactly for this reason, and voiced exactly these concerns when the proposal was first made, arguing instead that nomodule should only be added once import meta, dynamic import and module workers are implemented.

What we are left with is that we need to feature detect dynamic import first to do the fallback properly - basically bypassing nomodule. Yes that does mean that nomodule is useless...

@guybedford

This comment has been minimized.

Copy link
Contributor

@guybedford guybedford commented Jun 19, 2018

My original comment on the spec - whatwg/html#1442 (comment).

@guybedford

This comment has been minimized.

Copy link
Contributor

@guybedford guybedford commented Jun 19, 2018

@camille-hdl I've actually put some thought to this, and here's an approach that can work:

<!-- for older browsers -->
<script nomodule src='https://unpkg.com/systemjs@0.21.0/dist/system-production.js'></script>
<script nomodule>
  System.import('nomodule/main-a.js');
  System.import('nomodule/main-b.js');
</script>

<!-- for modern browsers -->
<!-- we import entrypoints with dynamic import so we can do a fallback if it isn't supported -->
<script type="module">
  window.esDynamicImport = true;
  import('/module/main-a.js');
  import('/module/main-b.js');
</script>

<!-- add this snippet to support browsers with modules but not dynamic import -->
<script type="module">
  if (!window.esDynamicImport) {
    const noms = Array.prototype.filter.call(document.getElementsByTagName('script'), x => x.hasAttribute('nomodule'));
    function nextLoad (nom, s) {
      if (!(nom = noms.shift())) return;
      s = document.createElement('script');
      if (nom.src)
        s.src = nom.src, s.addEventListener('load', nextLoad), s.addEventListener('error', nextLoad);
      else
        s.innerHTML = nom.innerHTML;
      document.head.appendChild(s);
      if (!nom.src) nextLoad();
    }
    nextLoad();
  }
</script>

Basically, you just have to alter the way you load the module entry points, and then add that extra fallback script to get this to work out as expected.

There will be some errors in the console for the module without dynamic import case, but execution will work out ok.

@guybedford

This comment has been minimized.

Copy link
Contributor

@guybedford guybedford commented Jun 19, 2018

I've posted a PR to the starter repo to encourage this as the default pattern in rollup/rollup-starter-code-splitting#5.

Thanks @camille-hdl for posting here.

@camille-hdl

This comment has been minimized.

Copy link
Author

@camille-hdl camille-hdl commented Jun 19, 2018

Thank you! I haven't done any extensive testing but I did try your suggestion and it seems to work for my use case, with this change:

- const noms = [...document.getElementsByTagName('script')].filter(x => x.hasAttribute('nomodule'));
+ const noms = Array.prototype.slice.call(document.getElementsByTagName('script'), 0).filter((x) => x.hasAttribute('nomodule'));

Apparently the spread operator doesn't work with HtmlCollection on Edge (17) and I didn't transpile.

Adding this to the starter repo is definitely a good idea for people like me unfamiliar with the nomodule pitfall and mostly using Chrome during development.

@guybedford

This comment has been minimized.

Copy link
Contributor

@guybedford guybedford commented Jun 19, 2018

Thanks for the feedback - have added the Edge fix.

Let's track the issue on the starter repo then. Closing this one now.

@guybedford guybedford closed this Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.