fix(babel-preset-gatsby): remove spread operator from node builds #29346
Conversation
gatsbot
bot
added
the
status: triage needed
wardpeet
added
status: needs core review
| @@ -114,7 +119,7 @@ export default function preset(_, options = {}) { | |||
| // absoluteRuntime: absoluteRuntimePath, | |||
| }, | |||
| ], | |||
| [ | |||
| isBrowser && [ | |||
There was a problem hiding this comment.
Will class-transform be provided when isBrowser is true? If not, Babel will still throw on super(...args). Before 7.12.11, babel outputs untranspiled super(...args) without any warnings.
There was a problem hiding this comment.
It will work unless people have a custom browserslist where spread is not needed 🤔. Good call
There was a problem hiding this comment.
Consider switch to assumptions after Babel 7.13 is released. Ideally what we need is if you enable spread-transform because of your specified targets, do not assume "iterable is array", which what the loose mode of spread-transform did. Here we are unconditionally transpiling ... even if the target already supports it.
There was a problem hiding this comment.
Awesome! Didn't know about assumptions yet. That will fix it. In the next major I'll add loose as a parameter so people can configure preset-env so we don't have to add a separate module for spread.
| isBrowser && [ | ||
| resolve(`@babel/plugin-transform-spread`), | ||
| { | ||
| loose: false, // Fixes #14848 | ||
| }, | ||
| ], | ||
| isBrowser && [ |
There was a problem hiding this comment.
Instead of transpiling both class and spread unconditionally, consider check if transform-spread is actually required for the given targets:
import { isRequired } from "@babel/helper-compilation-targets";
isRequired("transform-spread", targets)
// returns boolean, whether `transform-spread` is required.
// see https://unpkg.com/browse/@babel/compat-data@7.12.13/data/plugins.json for available feature namesYou can plug in transform-classes safely because if a target does not support ... natively, it does not support class either.
There was a problem hiding this comment.
TIL @babel/helper-compilation-targets. Thanks!
There was a problem hiding this comment.
@babel/helper-compilation-targets didn't accept targets how we handle it and pass it on to preset-env so I have to do some more digging and make it more resilient. I've made a ticket in our issue tracker
There was a problem hiding this comment.
@babel/preset-env uses isRequire under the hood: https://github.com/babel/babel/blob/74ed698c2e2140c0ef685b1542ef6ad8f20f0b20/packages/babel-preset-env/src/index.js#L288
It seems to me a bug of @babel/helper-compilation-targets if it didn't accept the targets passed to preset-env. Please file an issue to Babel if you can isolate the bug. Thanks!
Description
babel/babel#12722 Introduced a bug in Gatsby where ssr could not be built. We disabled spread operator in preset-env and added it manually to support non loose mode. Spread polyfilling is not necessary in node > 5 so disabling that fixes our issues.
Related Issues
Fixes #29326