-
Notifications
You must be signed in to change notification settings - Fork 622
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
Version 5.1.0 breaks react-vega #7437
Comments
Is this the same as vega/react-vega#367 (comment)? |
Ah, shoot. I think it is. Sorry, missed it in the other repo. |
Thanks for filing the issue so clearly so that I know what to look for. I see two solutions here. 1) Expect users to use babel 7.8 (https://babeljs.io/blog/2020/01/11/7.8.0) or 2) for now output es2017 instead of es 2020. If it's the latter, we need to fix that here. cc @kristw |
I feel cra is popular enough that more people run into this so it would make sense to emit es2015/2017. The issue comes from 275b53d. Note that bundles are unaffected which is why I thought this would be a safe upgrade. |
I'm wrong. Babel 7.8 doesn't seem to be enough since @mattdeitke's example uses babel 7.12 already. Maybe one needs to enable some plugin/transform to support optional chaining. @mattdeitke could you see whether you can enable support for optional chaining easily? Maybe relevant: facebook/create-react-app#8526 |
It looks like optional chaining is already supported by default in v3.3.0 of Here, there aren't any issues if I change function App()
const obj = {
foo: {
bar: 42,
},
};
if (obj.foo?.bar === 42) {
console.log("hello, world");
}
return <VegaLite spec={spec} data={barData} />;
} |
It could be that |
Using Create React App trying both JavaScript and TypeScript: JavaScriptJavaScript package.json: "react": "^17.0.2",
"react-scripts": "4.0.3",
"react-vega": "^7.4.3",
"vega": "^5.20.2",
"vega-lite": "^5.0.1", This gives error:
Then, tried the solution mentioned at vega/react-vega#367 (comment) by using react-app-rewired with Babel nullish coalescing and optional chaining plugins, but receive the same error as above. Since React supports optional chaining and nullish coalescing, you'd think this would work out of the box. JavaScript with Vega-Lite 5.0.0 downgradeNext, I've tried downgrading Vega Lite to 5.0.x as mentioned here, which initially produced the error referenced below in the TypeScript example. However, was able to get it to work when realizing it was changed tslib. Lock file diff, changing tslib: With this, I couldn't get "Approach #1" to work, per https://github.com/vega/react-vega/tree/master/packages/react-vega example. Maybe that's a few feature in 5.1? Using TypeScriptUsing npx to scaffold a TypeScript CRA also failed. Project boostrap:
TypeScript package.json "react": "^17.0.2",
"react-scripts": "4.0.3",
"react-vega": "^7.4.3",
"typescript": "^4.2.4",
"vega": "^5.20.2",
"vega-lite": "^5.0.1", Gives same error:
|
As said above, I'm not sure whether cra applies able to dependencies or only code itself. But I agree that it would be the right thing to do.
Will still resolve to 5.1. You need to set |
@domoritz Sorry, I posted the wrong version from package.json during my many tests. To confirm: 5.1.0 did not work, whereas 5.0.0 was functional - there's a screen capture of the diff above. Per node modules, I thought Create React App's environment presets included node modules in the frontend build pipeline. Their GitHub Troubleshooting guide states:
|
No problem. I'm still not sure we have a problem here then but it sounds like CRA is not doing what it says it does or am I missing something? |
Sorry for the late response here.
To clarify, the same issue (5.0.0 works, 5.1.0 fails) happens in other React major frameworks Gatsby & Next.js as well, so I think it's independent of CRA. (I hardly use CRA, but it was just the easiest to reproduce the example.)
This seems like a reasonable solution. I've tried a few claimed solutions to doing this online, although none of them seemed to work. I can try some more things over the next few days, have just gone back to 5.0.x in the meantime. (Also vega/react-vega#367 (comment) did not work.) Although, since |
That could be the case but I think that is problematic since it means that libraries would need to compile to the oldest possible JavaScript version. |
Hello, I have encountered with the same problem and read the discussion above, still feel confused about what the solution is. It would be appreciated to get replied! @jasonsturges |
CRA needs to update to Webpack 5. In the meantime, we will switch back to es2015 output. |
Summary
Hi,
I've tried upgrading several repos to use version 5.1.0, but it appears to break react-vega. Downgrading to version 5.0.x worked just as expected on all repos.
Webpack Error Message when using Create React App
[Long] Webpack error message when using Gatsby.js
Minimal Reproduction
Steps 1-3 are done in this repo. If it's easier, simply run:
Otherwise:
npx create-react-app my-app cd my-app
react-vega
dependencies:App.js
to the following minimal example that usesvega-lite
:Version 5.0.x Works
The text was updated successfully, but these errors were encountered: