fridays / next-routes

Universal dynamic routes for Next.js
MIT License
2.47k stars 230 forks source link

Eliminate use of Object.assign. Fixes #142 #143

Closed fritz-c closed 1 year ago

fritz-c commented 6 years ago

When the bundle was created by babel, calls to Object.assign were being left as-is, causing errors in IE 11 that left the user staring at a blank screen.

Fixes #142

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.03%) to 96.07% when pulling 247d447d550a716a3d325ba1087b4c72a3741254 on fritz-c:master into 773ea4d355ac7ac0cbf50a5dfd594e83e4429660 on fridays:master.

fridays commented 6 years ago

Thanks for contributing! I think it should be taken care of by next/babel. Do you use a custom babel file, and can you check if it's happening too with only a bare Next.js project?

fritz-c commented 6 years ago

I wasn't using a custom babel file. Here is an minimal reproduction repo: https://github.com/fritz-c/next-routes-ie

fritz-c commented 6 years ago

I suppose the issue with next/babel could be traced back to one of these changes: https://github.com/zeit/next.js/commits/ebf0c47c25541cfc50c8f319141f4bc9f47daeb8/server/build/babel/preset.js

stevenkissack commented 6 years ago

I am seeing the same issue in IE11, and my custom babel file is below: (I need it for the test framework)

{
    "env": {
      "development": {
        "presets": ["next/babel"]
      },
      "production": {
        "presets": ["next/babel"]
      },
      "test": {
        "presets": [["next/babel", { "preset-env": { "modules": "commonjs" } }]]
      }
    }
  }

Including a polyfill in my Layout component works for now but isn't optimal.

jonjamz commented 6 years ago

As stated in the Next.js docs, you should handle polyfills yourself. There is an example showing how to do it. See:

https://github.com/zeit/next.js#browser-support

Perhaps a warning in the docs would suffice?

stevenkissack commented 6 years ago

If we can easily switch out Object.assign and avoid babel configuration issues with no additional ongoing maintenance I'd argue that it makes sense? I'd forgotten I added the polyfill but this is something I'll be coming back to resolve soon, whether it be with babel or a fork of this repo.

crung commented 6 years ago

I agree with @stevokk. Let's just replace Object.assign with a local helper or something. Rather that than dabble with configuration.

oliviertassinari commented 5 years ago

This method isn't supported by Chrome 41 (googlebot!) and IE 11 (~3% of the global traffic). I think that it's important.