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

IE 11 breaking with Version 8.3.0 #516

Closed
KevinGruber opened this issue Sep 9, 2020 · 7 comments
Closed

IE 11 breaking with Version 8.3.0 #516

KevinGruber opened this issue Sep 9, 2020 · 7 comments

Comments

@KevinGruber
Copy link

Describe the bug

Since the minor version bump to 8.3.0 we experience problem sin IE11 with uuid.
Based on the error we get it pinpoints to the code which introduced there.
https://github.com/uuidjs/uuid/pull/479/files#diff-016d740d7835ede8d0af47fe304f7191R16
SCRIPT1006: ')' erwartet (expected)

How to reproduce

Use uuid with IE11.

Expected behavior

It doesn't throw an exception and stops code execution.

Runtime

  • OS: All
  • Runtime: IE11

Additional information

Works fine on other browsers.
We currently solved it with downgrading to 8.2.0.
But it took us quite some time to find the problem to be in uuid, as it was just a minor version bump.

Thanks for looking into it.

@ctavan
Copy link
Member

ctavan commented Sep 9, 2020

@KevinGruber this comes unexpected since the browser tests for IE11 did pass for the 8.3.0 release.

How exactly are you using uuid? Can you provide a minimal reproducible example for the error, including the respective module bundlers etc.? This would help a lot in further debugging the issue.

@KevinGruber
Copy link
Author

KevinGruber commented Sep 9, 2020

Hi @ctavan,

Of course:
We are using webpack 4.
In the bundled code it complains about an expected closing ")"

And i found it now with a really small repo.
You are using default value in stringify for parameter offset and that is not allowed in ES5 and lower (IE11).

IE11 repo.zip

Should I open a PR?
I could do it tomorrow

@ctavan
Copy link
Member

ctavan commented Sep 10, 2020

Hi @KevinGruber. Thanks for providing further information.

Unfortunately to me this still does not explain the issue. Normally, webpack should pick up the files from dist/esm-browser/ for browser builds, and these files have been transpiled to ES5 in order to work with IE11. Specifically, if you look at https://unpkg.com/browse/uuid@8.3.0/dist/esm-browser/stringify.js line 14 you can see that the default parameter statement has been converted into something that IE11 understands.

So my suspicion is that something about the webpack config might be odd. You could try the examples from https://github.com/uuidjs/uuid/tree/master/examples/browser-webpack (just run npm start and open the example.html file with IE11) and compare with your setup or post a complete example which involves your exact build config here.

@KevinGruber
Copy link
Author

KevinGruber commented Sep 10, 2020

Hi @ctavan,

i see what is "wrong", you specify browser as module keyword.
which we "import" last on differential build, because module is only es6 and higher.
then we look at main which includes the node version of your script which breaks.

our mainField order is: legacy, unpkg, browser, main, module for the es5 builds.

I also saw you have browser specified, but I am not familiar with that syntax, are you overriding just certain files?

I will try to change the order for main and module, but I fear it will break other packages.

@ctavan
Copy link
Member

ctavan commented Sep 10, 2020

OK, this explains it.

From your config the relevant mainField values are browser -> main -> module.

This will effectively pick the CommonJS builds from ./dist and the three CommonJS browser overrides that are specified in package.json.

I don't see a reason to specify main before module when using webpack though, because webpack will effectively resolve all imports at build time and bundle everything together, no need to worry about ES6 import syntax ever hitting a browser. I would in fact definitely advise to swap the order, because with the current setup you won't get the benefits of tree shaking that ES modules give you.

All that said you are still revealing a small inconsistency with this package since we did not intend to break compatibility of the CommonJS build for old browsers. I'll look into this but I still see a lot of benefits of switching the mainFields order (in fact I wouldn't even change webpacks defaults unless you absolutely have to, this usually works fine even when bundling for old browsers).

@KevinGruber
Copy link
Author

I am trying right now if it still works with the switched order,

We sadly need to change it, as we have internal libraries which we build for es5,es6 and cjs and those are then imported accordingly into our isomorphic application.

Thanks I will close the Ticket than

@ctavan
Copy link
Member

ctavan commented Sep 10, 2020

@KevinGruber one other idea would be to use babel-loader in order to transpile uuid, see https://stackoverflow.com/questions/54043498/how-to-transpile-node-modules-modules-with-babel-loader

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants