Skip to content

Fix parameters or JSX dev runtime#3880

Merged
JoviDeCroock merged 1 commit into
preactjs:masterfrom
wooorm:fix-jsx-dev
Feb 3, 2023
Merged

Fix parameters or JSX dev runtime#3880
JoviDeCroock merged 1 commit into
preactjs:masterfrom
wooorm:fix-jsx-dev

Conversation

@wooorm

@wooorm wooorm commented Feb 3, 2023

Copy link
Copy Markdown
Contributor

My guess is that the previous PR “fixed” the earlier problem because self isn’t used, so by calling isStaticChildrenself”, a bug went away.

The source for where this jsxDEV call is generated in Babel is here: https://github.com/babel/babel/blob/3952486/packages/babel-plugin-transform-react-jsx/src/create-plugin.ts#L506-L508.
The React RFC for the transform that mentions the dev runtime is here: https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md#dev-only-transforms.
React’s implementation is here: https://github.com/facebook/react/blob/855b77c9bbee347735efcd626dda362db2ffae1d/packages/react/src/jsx/ReactJSXElementValidator.js#L306-L311

* isStaticChildren is the same as whether jsxs would be used, instead of jsx. Which is also whether there are 2 or more children passed:

  • <a /> -> jsx('a', {})
  • <a>b</a> -> jsx('a', {children: 'b'})
  • <a>{1}{2}</a> -> jsxs('a', {children: [1, 2]})

Related-to: GH-3459.

* There was a parameter missing (`isStaticChildren: boolean`), which is not useful\*,
  but is still being passed
* Fix order of `source` and `self` again (incorrectly introduced in GH-3459)
* Fix some (internal JSDoc) types for these parameters

My guess is that the previous PR “fixed” the earlier problem because `self` isn’t used, so by
calling `isStaticChildren` “`self`”, a bug went away.

The source for where this `jsxDEV` call is generated in Babel is here:
<https://github.com/babel/babel/blob/3952486/packages/babel-plugin-transform-react-jsx/src/create-plugin.ts#L506-L508>.
The React RFC for the transform that mentions the dev runtime is here:
<https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md#dev-only-transforms>

\* `isStaticChildren` is the same as whether `jsxs` would be used, instead of `jsx`.
Which is also whether there are 2 or more children passed:

*   `<a />` -> `jsx('a', {})`
*   `<a>b</a>` -> `jsx('a', {children: 'b'})`
*   `<a>{1}{2}</a>` -> `jsxs('a', {children: [1, 2]})`

Related-to: GH-3459.
@wooorm

wooorm commented Feb 3, 2023

Copy link
Copy Markdown
Contributor Author

/cc @marvinhagemeister and @JoviDeCroock who worked on / reviewed the previous PR!

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage: 99.531%. Remained the same when pulling 56452dd on wooorm:fix-jsx-dev into fc5758b on preactjs:master.

@JoviDeCroock JoviDeCroock merged commit 5eecaf1 into preactjs:master Feb 3, 2023
@JoviDeCroock

Copy link
Copy Markdown
Member

@wooorm thank you for all the pointers, made it much easier to review this!

@marvinhagemeister

Copy link
Copy Markdown
Member

@wooorm Good catch, that's on me for messing up the arguments. Thank you so much for filing a PR!

@wooorm wooorm deleted the fix-jsx-dev branch February 3, 2023 11:36
JoviDeCroock added a commit that referenced this pull request Jan 12, 2024
JoviDeCroock added a commit that referenced this pull request Jan 12, 2024
* backport #3871

* port test from #3884 functionality seems to work in v11

* backport #3880

* backport #3875

* backport #3862

* add todo for #3801

* backport #3868

* backport #3867

* backport #3863

* add todo for #3856

* backport #3844

* backport #3816

* backport #3888

* backport #3889
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

Successfully merging this pull request may close these issues.

4 participants