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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

IE (Internet Explorer) Error 馃挬 #473

Closed
philcockfield opened this issue Dec 22, 2016 · 13 comments 路 Fixed by threepointone/glamor#166
Closed

IE (Internet Explorer) Error 馃挬 #473

philcockfield opened this issue Dec 22, 2016 · 13 comments 路 Fixed by threepointone/glamor#166
Labels
Upstream Related to using Next.js with a third-party dependency. (e.g., React, UI/icon libraries, etc.).

Comments

@philcockfield
Copy link

philcockfield commented Dec 22, 2016

Running a simple page that references nothing other than React within Next.js on IE9...11 causes the following error:

image

This occurs with both next.js version 1.2.3 and 2.0.0-beta.0.

An example of this on IE9 can be seen here (courtesy of @arunoda):

https://www.browserling.com/browse/win/vista/ie/9/https%3A%2F%2F37a02b6c.ngrok.io%2Fabout

The code which is throwing the error seems related to stylesheets:

Object.assign(StyleSheet.prototype, {
  getSheet: function getSheet() {
    return sheetForTag(last(this.tags));
  },
  inject: function inject() {
    var _this = this;

    if (this.injected) {
      throw new Error('already injected stylesheet!');
    }
    if (isBrowser) {
      this.tags[0] = makeStyleTag();
    } else {
  ...

Object.assign is not in ES5 and so this is perhaps a babel related problem where an external library has been referenced that has not been transpiled.

This will not be showing up as an error in Chrome/Firefox (and all the other "big boy browsers") as they have started to implement some of the ES6 spec natively.

@philcockfield
Copy link
Author

philcockfield commented Dec 22, 2016

TEMPORARY HACK. To work around this until it's fixed you can add the babel-polyfill to the page:

    <script
      type='text/javascript'
      src='https://cdnjs.cloudflare.com/ajax/libs/babel-polyfill/6.20.0/polyfill.min.js' />

@arunoda
Copy link
Contributor

arunoda commented Dec 22, 2016

Here's the source of the issue: threepointone/glamor#155

@luisrudge
Copy link

I think polyfills should be added manually if the dev needs to support old browsers

@arunoda
Copy link
Contributor

arunoda commented Dec 22, 2016

@luisrudge actually it's a problem of glamor. It's not doing babel transpiling properly.

@philcockfield
Copy link
Author

@luisrudge I agree with @arunoda. Object.assign transpiles via babel just fine when used within a consuming project. But it's the NextJS bundle itself which is delivering some un-transpiled code within it.

@arunoda
Copy link
Contributor

arunoda commented Dec 22, 2016

@philcockfield I already send a PR for glamor and I hope they'll accept it and cut a new release.
See: threepointone/glamor#166

@philcockfield
Copy link
Author

Rock and roll @arunoda! Awesome 馃挴

chiefjester added a commit to chiefjester/next.js that referenced this issue Dec 25, 2016
- fixes vercel#473
- this polyfill is very small as compared to alternatives
- just need to add it as a plugin in babel
@chiefjester
Copy link
Contributor

@arunoda next.js is using Object.assign. It's under the utils function https://github.com/zeit/next.js/blob/master/lib/utils.js#L20, the #513 fixes this with just adding a simple transform.

I do believe this fix #513 is better instead of adding the whole babel-polyfill as a dependency. babel-polyfill is actually pretty big, and adding that should be on the userland instead of the library.

This same fix applies in glamor, instead of adding babel-polyfill, its better to add this transform.

@arunoda
Copy link
Contributor

arunoda commented Dec 25, 2016

@thisguychris yes Next.js uses Object.assign and many other ES2015+ features.
Actually, babel's transform runtime is pretty clever and it doesn't include the whole shims/polyfills.

Check the transpiled code of the above mentioned utils.js

So, the issue is not in the Next.js code base and our use of babel-runtime is pretty efficient. If glamor didn't fix this issue, we may need to hot fix it.

@chiefjester
Copy link
Contributor

@arunoda oh that's good to know! looks like they fixed it in v6 and up, Our app is still in v5 馃槀

@arunoda arunoda added the Upstream Related to using Next.js with a third-party dependency. (e.g., React, UI/icon libraries, etc.). label Dec 26, 2016
@arunoda
Copy link
Contributor

arunoda commented Dec 27, 2016

Guys, now glamor has a fix for this issue.
We've bumped the glamor version in master and it should work properly.

But Next.js apps only work on IE9+ versions only.
That's because of our use of history API.

@arunoda
Copy link
Contributor

arunoda commented Dec 30, 2016

I hope we could close this now.
See^^^

@arunoda arunoda closed this as completed Dec 30, 2016
@philcockfield
Copy link
Author

馃憤

@lock lock bot locked as resolved and limited conversation to collaborators May 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Upstream Related to using Next.js with a third-party dependency. (e.g., React, UI/icon libraries, etc.).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants