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

New JSX runtime use adds spread overhead #2773

Closed
bz2 opened this issue Sep 24, 2020 · 3 comments
Closed

New JSX runtime use adds spread overhead #2773

bz2 opened this issue Sep 24, 2020 · 3 comments

Comments

@bz2
Copy link
Contributor

bz2 commented Sep 24, 2020

Using new JSX runtime compilation through babel added in preact 10.5.x adds spread operations and overhead in cases the old createElement-like interface did not have.

For instance, one chunk in a project I'm working on went from 30.9 KiB minified to 34.6 KiB. It looks like the actual fix(es) will need to happen at the babel level, but might be worth documenting a sane path for now with preact?

Steps

Clone https://github.com/bz2/example-babel-jsx-spread for a minimal reproduction.

For a component like this:

function App({style}) {
  return <h1 {...{style}}>Title</h1>;
}

Using a pragma and importing h results in babel with @babel/preset-env generating the following code:

function App(_ref) {
  var style = _ref.style;
  return h("h1", {
    style: style
  }, "Title");
}

Whereas using '@babel/plugin-transform-react-jsx', {runtime: 'automatic', importSource: 'preact'}] produces:

function App(_ref) {
  var style = _ref.style;
  return jsxRuntime_module_o("h1", _objectSpread(_objectSpread({}, {
    style: style
  }), {}, {
    children: "Title"
  }));
}

Note this case of explicit spreading a literal is particularly bad because spread is actually known to not be needed at compile time.

The switch to using spread was added in babel/babel#10572 and this is also gets made worse by babel/babel#10261 (has a solution, but is non-obvious as a problem) which resulted in a local copy of _objectSpread and its helpers in every file.

In preset-modules babel/preset-modules#4 was used to use Object.assign instead, but I don't see how to get that to work for the remerged bugfixes: true option of preset-env.

Expected Behavior

Using the new JSX runtime should result in faster, and also not larger code.

Actual Behavior

Using the new JSX runtime generates larger, slower code.

Solution

Probably a bit more smarts upstream in the babel-helper-builder-react-jsx-experimental and some new docs in preact showing the right combination of babel options for the best results?

@marvinhagemeister
Copy link
Member

marvinhagemeister commented Sep 24, 2020

The actual transformation happens in the babel plugin, so the babel bug tracker is the proper place to file this. What we do in our preact/jsx-runtime entry is to supply the various jsx functions. Anything related to _objectSpread sounds more like something injected by babel for older browsers.

Agree that the output generated by babel can be improved. We've been sending some patches upstream as a start.

Closing since there is nothing we can do about that in Preact. It's an issue with the babel plugin hosted over at the babel repo, it affects React users too.

@developit
Copy link
Member

Hi @bz2 - @babel/plugin-transform-react-jsx has an option that uses Object.assign() for JSX spread rather than the helper. I've sent you a PR here:

bz2/example-babel-jsx-spread#1

@bz2
Copy link
Contributor Author

bz2 commented Sep 25, 2020

@developit Thanks! Pointers in the right direction greatly appreciated. Responding with some questions on that PR.

@marvinhagemeister I'm not quite sure what changes still need to happen upstream in babel, but can report/pr a doc change to preact once the sensible ways of specifying the transpilaton options are clear?

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

No branches or pull requests

3 participants