[transform-react-jsx] Add useSpread option to transform JSX by ivandevp · Pull Request #10572 · babel/babel

ivandevp · 2019-10-18T04:32:17Z

Q	A
Fixed Issues?	Fixes #9652
Patch: Bug Fix?	No
Major: Breaking Change?	No
Minor: New Feature?	Yes
Tests Added + Pass?	Yes
Documentation PR Link	babel/website#2110
Any Dependency Changes?	No
License	MIT

buildsize · 2019-10-18T14:05:39Z

File name	Previous Size	New Size	Change
babel-preset-env.js	2.73 MB	2.73 MB	9 bytes (0%)
babel-preset-env.min.js	1.65 MB	1.65 MB	9 bytes (0%)
babel.js	2.92 MB	2.92 MB	421 bytes (0%)
babel.min.js	1.61 MB	1.61 MB	245 bytes (0%)
test262.tap	4.84 MB	[deleted]

nicolo-ribaudo

Can you also add a test where the spread isn't the first property?

nicolo-ribaudo · 2019-10-18T16:00:04Z

    let _props = [];
    const objs = [];

+    const useSpread = file.opts.useSpread || false;


I don't like that this accepts 0, but let's keep it as-is for consistency with the other options of this file 🤷‍♀️

We should revisit it in Babel 8.

IMO, we could begin on useSpread now because it is a new option. useBuiltins: 0 is strange enough and it does not justify that you can even use useSpread: 0.

JLHwung

I think when useSpread is enabled, we can further simplify the logic to

return attribs.map(convertAttribute)

because objs is used to support breaking at spreadElement only.

And we may teach convertAttribute to replace JSXSpreadAttribute by spreadElement.

JLHwung · 2019-10-18T21:41:20Z

    let _props = [];
    const objs = [];

+    const useSpread = file.opts.useSpread || false;


I think we should also add useSpread to preset-react, too.

Another thought is that may be we should give a warning (or even error) when both useBuiltIns and useSpread are true because useSpread precedes useBuiltIns.

…-spread

…nto feat/use-spread

nicolo-ribaudo · 2019-10-25T22:22:05Z

+      );
+    }
+
+    const useBuiltIns = setDefaultValue(file.opts.useBuiltIns, false);


While this makes 100% sense, it is a breaking change (for example, if you pass 0) and we should defer it to Babel 8.

So. Should I go back to the previous version?

Yes 👍 but only here, not for useSpread.

Oh ok, got it.

nicolo-ribaudo · 2019-10-25T22:22:41Z

    const objs = [];

-    const useBuiltIns = file.opts.useBuiltIns || false;
+    const useSpread = setDefaultValue(file.opts.useSpread, false);


You can use

const { useSpread = false } = file.opts;

…-spread

ivandevp · 2019-10-27T14:59:20Z

I think when useSpread is enabled, we can further simplify the logic to
return attribs.map(convertAttribute)
because objs is used to support breaking at spreadElement only.

And we may teach convertAttribute to replace JSXSpreadAttribute by spreadElement.

Hey @JLHwung , do you mean to add a condition before while loop to check if useSpread is set and put an early return mapping all attributes to convertAttribute? If that's the case, I would need to return and object expression instead of an array, right? Something like:

if (useSpread) {
  const props = attribs.map(convertAttribute);
  return t.objectExpression(props);
}

JLHwung · 2019-10-27T15:26:06Z

@ivandevp correct, I missed the objectExpression but you got me.

ivandevp · 2019-10-28T16:42:13Z

I've made the requested changes guys, let me know if you think we can do any other enhancement.

JLHwung · 2019-10-28T17:18:28Z

@ivandevp Could you add useSpread to preset-react, too? The other changes look good to me.

ivandevp · 2019-10-28T18:14:59Z

Sure @JLHwung! I forgot that one 😅

…-spread

ivandevp · 2019-10-29T02:08:37Z

Hey @JLHwung ! I've tried to follow the useBuiltIns flow in preset-react in order to do the same for useSpread. Is my change ok?

nicolo-ribaudo

The CI failure is unrelated

Co-Authored-By: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

A new `useSpread` option was added in Babel 7.7.0 (babel/babel#10572): https://babeljs.io/docs/en/babel-preset-react#usespread Enabling this means that the JSX-> JS conversion will use an inline object with spread elements (for code that spreads props), rather than Babel's extend helper or `Object.assign`. `@babel/env` will still transpile these down if required for the target browsers (the only reason `useSpread` is not already enabled by default is that for the upstream project this is a breaking change, since they cannot assume that users of `babel-preset-react` are always using it alongside `@babel/env`, even though most likely are).

ivandevp added 2 commits October 17, 2019 23:30

[transform-react-jsx] Add useSpread option to transform JSX

345654d

Merge branch 'master' into feat/use-spread

c547265

nicolo-ribaudo added area: jsx PR: New Feature 🚀 A type of pull request used for our changelog categories labels Oct 18, 2019

ivandevp mentioned this pull request Oct 18, 2019

[transform-react-jsx] Add useSpread option doc babel/website#2110

Merged

nicolo-ribaudo reviewed Oct 18, 2019

View reviewed changes

JLHwung reviewed Oct 18, 2019

View reviewed changes

ivandevp added 4 commits October 22, 2019 19:15

Merge branch 'master' of https://github.com/babel/babel into feat/use…

48d6c09

…-spread

Merge branch 'master' of https://github.com/babel/babel into feat/use…

51fd79d

…-spread

Add validation for default option

60aad23

Merge branch 'feat/use-spread' of https://github.com/ivandevp/babel i…

3afabbf

…nto feat/use-spread

nicolo-ribaudo reviewed Oct 25, 2019

View reviewed changes

ivandevp added 2 commits October 27, 2019 08:38

Merge branch 'master' of https://github.com/babel/babel into feat/use…

d905397

…-spread

Add error when using useSpread and useBuiltIns at the same time

617dd72

Move useSpread to convertAttribute helper function

3e32a4e

ivandevp added 2 commits October 28, 2019 20:04

Merge branch 'master' of https://github.com/babel/babel into feat/use…

ec9f24b

…-spread

Add useSpread option to presect-react

7b0f831

JLHwung approved these changes Oct 29, 2019

View reviewed changes

nicolo-ribaudo added this to the v7.7.0 milestone Oct 29, 2019

nicolo-ribaudo approved these changes Oct 29, 2019

View reviewed changes

nicolo-ribaudo reviewed Oct 29, 2019

View reviewed changes

Comment thread packages/babel-preset-react/src/index.js Outdated

Remove casting useSpread to boolean in preset-react option.

35ec82b

Co-Authored-By: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

nicolo-ribaudo added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label Oct 29, 2019

nicolo-ribaudo merged commit 3d2f365 into babel:master Oct 29, 2019

edmorley mentioned this pull request Nov 10, 2019

react: Use babel-preset-react's new useSpread option neutrinojs/neutrino#1488

Merged

snyk-bot mentioned this pull request Nov 14, 2019

[Snyk] Upgrade @babel/runtime from 7.6.3 to 7.7.1 cds-snc/report-a-cybercrime#936

Merged

nicolo-ribaudo mentioned this pull request Nov 21, 2019

Babel 8 Release Plan #10746

Open

34 tasks

This was referenced Nov 28, 2019

[Snyk] Upgrade @babel/runtime from 7.0.0-beta.42 to 7.7.2 ali8889/mist#4

Open

[Snyk] Upgrade @babel/runtime from 7.5.5 to 7.7.4 dyna-dot/CyberChef#3

Open

This was referenced Dec 5, 2019

[Snyk] Upgrade @babel/runtime from 7.3.1 to 7.7.4 dyna-dot/bit-javascript#5

Open

[Snyk] Upgrade @babel/runtime from 7.5.5 to 7.7.4 jakeherp/todo-list#1

Merged

snyk-bot mentioned this pull request Jan 12, 2020

[Snyk] Upgrade @babel/runtime from 7.4.4 to 7.7.7 mboocannon/msgraph-sdk-javascript#1

Open

lock Bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 29, 2020

lock Bot locked as resolved and limited conversation to collaborators Jan 29, 2020

Uh oh!

Conversation

ivandevp commented Oct 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

buildsize Bot commented Oct 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Oct 18, 2019

Choose a reason for hiding this comment

Uh oh!

JLHwung Oct 18, 2019

Choose a reason for hiding this comment

Uh oh!

JLHwung left a comment

Choose a reason for hiding this comment

Uh oh!

JLHwung Oct 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Oct 25, 2019

Choose a reason for hiding this comment

Uh oh!

ivandevp Oct 27, 2019

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Oct 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivandevp Oct 27, 2019

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Oct 25, 2019

Choose a reason for hiding this comment

Uh oh!

ivandevp commented Oct 27, 2019

Uh oh!

JLHwung commented Oct 27, 2019

Uh oh!

ivandevp commented Oct 28, 2019

Uh oh!

JLHwung commented Oct 28, 2019

Uh oh!

ivandevp commented Oct 28, 2019

Uh oh!

ivandevp commented Oct 29, 2019

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ivandevp commented Oct 18, 2019 •

edited

Loading

buildsize Bot commented Oct 18, 2019 •

edited

Loading

JLHwung Oct 18, 2019 •

edited

Loading

nicolo-ribaudo Oct 27, 2019 •

edited

Loading