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

es5-build file has spread operator in code (and erroring in IE 11) #684

Closed
pydolan opened this issue May 10, 2021 · 6 comments
Closed

es5-build file has spread operator in code (and erroring in IE 11) #684

pydolan opened this issue May 10, 2021 · 6 comments

Comments

@pydolan
Copy link
Contributor

pydolan commented May 10, 2021

I'm seeing a "Syntax error" at line 11, column 3240, which in the minified file - https://cdn.jsdelivr.net/npm/vega-embed@6/build-es5/vega-embed.min.js - is the spread operator in this code:

/\bsemver\b/i.test(process.env.NODE_DEBUG)?(...e)=>console.error("SEMVER",...e):()=>{},G=function(

Or in the non-minified file - https://cdn.jsdelivr.net/npm/vega-embed@6.17.0/build-es5/vega-embed.js - looks like the code is this:

 const debug = (
    typeof process === 'object' &&
    process.env &&
    process.env.NODE_DEBUG &&
    /\bsemver\b/i.test(process.env.NODE_DEBUG)
  ) ? (...args) => console.error('SEMVER', ...args)
    : () => {};

Is some 3rd party code not being transpiled?

@domoritz
Copy link
Member

We shouldn't even need any code to check for node. Can you help fix this?

@pydolan
Copy link
Contributor Author

pydolan commented May 10, 2021

I haven't used rollup.js before, but after looking at the vega project's rollup, I wonder if the babel plugin would help here: https://github.com/vega/vega/blob/master/rollup.config.js

@domoritz
Copy link
Member

The es5 build is actually defaults from browserlist, which corresponds to https://browserslist.dev/?q=ZGVmYXVsdHM%3D, which should not support spread syntax today: https://caniuse.com/?search=spread. We use https://github.com/wessberg/rollup-plugin-ts so I guess the error might be there.

I will remove es5 bundles in the next release anyway so I won't have the cycles to investigate this further. Do you have time to fix this issue?

@pydolan
Copy link
Contributor Author

pydolan commented May 10, 2021

Understandable about dropping the ES5 support. I'll see what I can do about resolving this issue.

I played around with it a bit, and I think the project's babel.config.js is being used behind the scenes by rollup (of course, it could be ignorance on my part about how this works).

I tried adding this to the ts() config, but then I ran into the "deoptimizePath" issue you posted about on here:

    transpiler: "babel",
    babelConfig: {"presets": ["@babel/preset-env"], "targets": "defaults"},

@domoritz
Copy link
Member

Ohhh, that would make sense. We need the babel config for jest so we should ignore that for the build.

To fix rollup/rollup#4067, you can use an older version of rollup. Feel free to pin to a previous version for now.

@domoritz
Copy link
Member

Fixed in #689

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

No branches or pull requests

2 participants