-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Babel plugin JSX: Implement Fragment handling #15120
Conversation
111da78
to
8c88a7a
Compare
Should the preset be updated with the recommended config as well? gutenberg/packages/babel-preset-default/index.js Lines 57 to 67 in 354c1ff
|
Sure, why not. If we start using it in Gutenberg, it should be exposed to everyone. It will require adding a note to the changelog file. It seems to be a new feature, right? |
I've pushed a couple of commits that update the codebase to exercise the new functionality for handling JSX I'm happy to roll back those commits or break them out into another PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to roll back those commits or break them out into another PR.
Technically I'd think they'd be better for a separate pull request, though I don't really feel strongly about it. You may encounter some issue with merge conflicts if it's left to review for much longer.
} | ||
|
||
return state._options; | ||
} | ||
|
||
return { | ||
visitor: { | ||
JSXElement( path, state ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why this was changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I noticed that without this change, the following test fails (added in this PR):
it( 'adds import for scope variable for Fragments', () => {
const original = 'let foo = <></>;';
const string = getTransformedCode( original );
expect( string ).toBe( 'import React from "react";\nlet foo = <></>;' );
} );
● babel-plugin-import-jsx-pragma › adds import for scope variable for Fragments
expect(received).toBe(expected) // Object.is equality
Expected: "import React from \"react\";
let foo = <></>;"
Received: "let foo = <></>;"
27 |
28 |
> 29 | expect( string ).toBe( 'import React from "react";\nlet foo = <></>;' );
| ^
30 | } );
31 |
32 | it( 'does nothing if there is no jsx', () => {
at Object.toBe (packages/babel-plugin-import-jsx-pragma/test/index.js:29:20)
It seems that JSX
is an alias for most or all JSX that may be encountered.
It seems that <></>
should cause the import to be added.
source: '@wordpress/element', | ||
isDefault: false, | ||
}, | ||
], | ||
[ require.resolve( '@babel/plugin-transform-react-jsx' ), { | ||
pragma: 'createElement', | ||
pragmaFrag: 'Fragment', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re: Abbreviations. I suppose part of the purpose is in inheriting the Babel option by the same name?
https://babeljs.io/docs/en/babel-plugin-transform-react-jsx#pragmafrag
A bit unfortunate naming. I guess I'd rather be consistent than avoid the abbreviation, though.
commit 1dddab4 Author: Stefanos Togkoulidis <[email protected]> Date: Wed May 1 00:00:50 2019 +0300 Have Aztec delete the detected Enter key for paragraphs Aztec-Android doesn't swallow the Enter key (like the list handling does) so, instruct Aztec to delete it for the paragraph block. commit 0936ca0 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 30 20:58:36 2019 +0300 Just use onFormatChange which now defaults to "force" commit 6358de3 Merge: a5282ce 5e4d627 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 30 20:23:57 2019 +0300 Merge branch 'master' into rnmobile/fix-list-handling-android commit 5e4d627 Author: Pascal Birchler <[email protected]> Date: Tue Apr 30 18:27:24 2019 +0200 Update to Babel 7.4 and core-js 3 (#15139) * Specify core-js version when using useBuiltIns * Update to Babel 7.4 and core-js 3 * setupFiles: fix path to async-iterator * Also update Babel packages in babel-preset-default * Update snapshot for babel-preset-default test * Manually include web.dom-collections.iterator in jest config * Regenerate package-lock.json file after latest changes applied * Upgrade ESLint related npm packages * Another cleanup in the package-lock.json file * Remove ESLint rules which throw error to fix linting * Add core-js/modules/web.dom.iterable to make e2e tests work * Update Babel packages to the latest version * Update ignore files in e2e tests config * Add changelog entries related to Babel and ESLint version bumps commit 1d959ba Author: Andrés <[email protected]> Date: Tue Apr 30 17:14:35 2019 +0200 Remove not used state (#15224) commit d715e93 Author: Jon Surrell <[email protected]> Date: Tue Apr 30 16:29:09 2019 +0200 Babel plugin JSX: Implement Fragment handling (#15120) Add imports for `<></>` JSX Fragments. commit 96cad99 Author: Kelly Dwan <[email protected]> Date: Tue Apr 30 09:18:54 2019 -0400 Fix incorrect ID in FocalPointPicker (#15255) commit c1ba13e Author: Jon Surrell <[email protected]> Date: Tue Apr 30 13:44:50 2019 +0200 Update Fragment imports in READMEs (#15262) Some Fragment imports were incorrect. Fix them. commit 297c2f4 Author: etoledom <[email protected]> Date: Tue Apr 30 13:19:34 2019 +0200 RNMobile: Fix autoscroll on ListBlock (#15048) * RNMobile: Add ability to send extra props (i.e. props needed just for mobile) to the list block. * Adding onCaretVerticalPositionChange to RichText via context * Passing onCaretVerticalPositionChange via context to RichText for all RichText based blocks This makes it not necessary to pass onCaretVerticalPositionChange as a prop directly to RichText from the block component. * Update jest snapshot for block-edit commit 447dd27 Author: Jorge Costa <[email protected]> Date: Tue Apr 30 10:47:55 2019 +0100 Fix copy paste and delete error core paragraph with locking (#14712) * Fix: Copy / Paste error core/paragraph with locking * Add test case. * Update packages/block-library/src/paragraph/edit.js Co-Authored-By: jorgefilipecosta <[email protected]> commit 0b2eb1c Author: Jon Surrell <[email protected]> Date: Tue Apr 30 09:39:29 2019 +0200 Dependency extraction webpack plugin: Change option name consistent with docs (#15260) In the docs, an option name was provided as `requestToHandle`, but the actual option in use was `requestToDependency`. Update the code and related tests to use `requestToHandle` as described in the documentation instead of `requestToDependency`. commit 506827e Author: Jon Surrell <[email protected]> Date: Tue Apr 30 09:31:38 2019 +0200 Read script dependencies from generated files (#15124) * Rework script registration to use generated deps * Deprecate static list of dependencies * Rework translation without packages-dependencies.php * Include build/*/*.deps.json in plugin zip * Add script dependencies undetectable by build tools Some scripts have dependencies that are undetectable by the webpack plugin used to generate the dependency files. Add these dependencies to the generated dependencies. * Add `wp-polyfill` dependency via webpack plugin Enable `injectPolyfill` option in the webpack plugin instead of manually injecting the dependency via PHP on script registration. This will enable the polyfill for consumers of wp-scripts default webpack config. * Improve deprecated message * Remove obsolete lib/packages-dependencies.php commit c2c8276 Author: Daniel Richards <[email protected]> Date: Tue Apr 30 13:36:49 2019 +0800 Use button block appender on group block (#14943) * Use button block appender on group block * Update group block e2e tests for the new button appender * Add additional margin to the appender when a group has a background color. * Use more opaque versions of the appender background color To ensure greater compatibility when layered on top of a block with a background color. * Block Library: Display ButtonBlockAppender only if selected or empty * Only render button appender when the group has no inner blocks commit 1e96ec7 Author: Andrew Duthie <[email protected]> Date: Mon Apr 29 16:40:24 2019 -0400 Build Tooling: Pass individual files as arguments from watch to build script (#15219) commit cafb57b Author: Grzegorz (Greg) Ziółkowski <[email protected]> Date: Mon Apr 29 22:12:08 2019 +0200 Blocks: Upgrade simple-html-tokenizer dependency (#15246) commit 5b3b3ab Author: Andrew Duthie <[email protected]> Date: Mon Apr 29 13:38:09 2019 -0400 Framework: Add REST API codeowners (#15215) commit 64c48fd Author: Ian Dunn <[email protected]> Date: Mon Apr 29 10:06:46 2019 -0700 Correct name of `date_i18n()` PHP function. (#15204) commit 8c28814 Author: Andrew Duthie <[email protected]> Date: Mon Apr 29 13:05:10 2019 -0400 Framework: Remove TESTS.md (#15217) commit 0f1fb57 Author: Emmanuel Hesry <[email protected]> Date: Mon Apr 29 17:34:22 2019 +0200 fix typo in withDispatch documentation (#15251) commit a5282ce Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 16:08:31 2019 +0300 Revert "Trivial change to trigger Travis" This reverts commit e22ffde. commit e22ffde Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 16:08:24 2019 +0300 Trivial change to trigger Travis commit 15a51f8 Author: Jorge Costa <[email protected]> Date: Fri Apr 19 15:33:15 2019 +0100 chore: Fix: Lint error that makes unit tests (and CI tests) fail. (#15073) commit 2b11074 Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 12:42:55 2019 +0300 Fix lint issues commit 9c298c8 Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 12:01:46 2019 +0300 Need to specify firedAfterTextChanged on all Aztec events commit 46e086d Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 03:59:59 2019 +0300 Force Aztec update if "Enter" fired before text change commit 724e295 Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 03:11:52 2019 +0300 Differentiate Android and iOS since assumptions diverged The iOS side still expects to just check against `this.lastContent` to force the change into Aztec. commit 52386fa Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 01:55:22 2019 +0300 Use a flag to signal Aztec-originated changes And assume that when that flag is false, component changes need to get sent/reflected down to Aztec. commit c7aa381 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 23 17:30:45 2019 +0300 Able to not lose content commit 60c75b0 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 23 10:09:23 2019 +0300 If text already changed, don't modify it
commit 1dddab4 Author: Stefanos Togkoulidis <[email protected]> Date: Wed May 1 00:00:50 2019 +0300 Have Aztec delete the detected Enter key for paragraphs Aztec-Android doesn't swallow the Enter key (like the list handling does) so, instruct Aztec to delete it for the paragraph block. commit 0936ca0 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 30 20:58:36 2019 +0300 Just use onFormatChange which now defaults to "force" commit 6358de3 Merge: a5282ce 5e4d627 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 30 20:23:57 2019 +0300 Merge branch 'master' into rnmobile/fix-list-handling-android commit 5e4d627 Author: Pascal Birchler <[email protected]> Date: Tue Apr 30 18:27:24 2019 +0200 Update to Babel 7.4 and core-js 3 (#15139) * Specify core-js version when using useBuiltIns * Update to Babel 7.4 and core-js 3 * setupFiles: fix path to async-iterator * Also update Babel packages in babel-preset-default * Update snapshot for babel-preset-default test * Manually include web.dom-collections.iterator in jest config * Regenerate package-lock.json file after latest changes applied * Upgrade ESLint related npm packages * Another cleanup in the package-lock.json file * Remove ESLint rules which throw error to fix linting * Add core-js/modules/web.dom.iterable to make e2e tests work * Update Babel packages to the latest version * Update ignore files in e2e tests config * Add changelog entries related to Babel and ESLint version bumps commit 1d959ba Author: Andrés <[email protected]> Date: Tue Apr 30 17:14:35 2019 +0200 Remove not used state (#15224) commit d715e93 Author: Jon Surrell <[email protected]> Date: Tue Apr 30 16:29:09 2019 +0200 Babel plugin JSX: Implement Fragment handling (#15120) Add imports for `<></>` JSX Fragments. commit 96cad99 Author: Kelly Dwan <[email protected]> Date: Tue Apr 30 09:18:54 2019 -0400 Fix incorrect ID in FocalPointPicker (#15255) commit c1ba13e Author: Jon Surrell <[email protected]> Date: Tue Apr 30 13:44:50 2019 +0200 Update Fragment imports in READMEs (#15262) Some Fragment imports were incorrect. Fix them. commit 297c2f4 Author: etoledom <[email protected]> Date: Tue Apr 30 13:19:34 2019 +0200 RNMobile: Fix autoscroll on ListBlock (#15048) * RNMobile: Add ability to send extra props (i.e. props needed just for mobile) to the list block. * Adding onCaretVerticalPositionChange to RichText via context * Passing onCaretVerticalPositionChange via context to RichText for all RichText based blocks This makes it not necessary to pass onCaretVerticalPositionChange as a prop directly to RichText from the block component. * Update jest snapshot for block-edit commit 447dd27 Author: Jorge Costa <[email protected]> Date: Tue Apr 30 10:47:55 2019 +0100 Fix copy paste and delete error core paragraph with locking (#14712) * Fix: Copy / Paste error core/paragraph with locking * Add test case. * Update packages/block-library/src/paragraph/edit.js Co-Authored-By: jorgefilipecosta <[email protected]> commit 0b2eb1c Author: Jon Surrell <[email protected]> Date: Tue Apr 30 09:39:29 2019 +0200 Dependency extraction webpack plugin: Change option name consistent with docs (#15260) In the docs, an option name was provided as `requestToHandle`, but the actual option in use was `requestToDependency`. Update the code and related tests to use `requestToHandle` as described in the documentation instead of `requestToDependency`. commit 506827e Author: Jon Surrell <[email protected]> Date: Tue Apr 30 09:31:38 2019 +0200 Read script dependencies from generated files (#15124) * Rework script registration to use generated deps * Deprecate static list of dependencies * Rework translation without packages-dependencies.php * Include build/*/*.deps.json in plugin zip * Add script dependencies undetectable by build tools Some scripts have dependencies that are undetectable by the webpack plugin used to generate the dependency files. Add these dependencies to the generated dependencies. * Add `wp-polyfill` dependency via webpack plugin Enable `injectPolyfill` option in the webpack plugin instead of manually injecting the dependency via PHP on script registration. This will enable the polyfill for consumers of wp-scripts default webpack config. * Improve deprecated message * Remove obsolete lib/packages-dependencies.php commit c2c8276 Author: Daniel Richards <[email protected]> Date: Tue Apr 30 13:36:49 2019 +0800 Use button block appender on group block (#14943) * Use button block appender on group block * Update group block e2e tests for the new button appender * Add additional margin to the appender when a group has a background color. * Use more opaque versions of the appender background color To ensure greater compatibility when layered on top of a block with a background color. * Block Library: Display ButtonBlockAppender only if selected or empty * Only render button appender when the group has no inner blocks commit 1e96ec7 Author: Andrew Duthie <[email protected]> Date: Mon Apr 29 16:40:24 2019 -0400 Build Tooling: Pass individual files as arguments from watch to build script (#15219) commit cafb57b Author: Grzegorz (Greg) Ziółkowski <[email protected]> Date: Mon Apr 29 22:12:08 2019 +0200 Blocks: Upgrade simple-html-tokenizer dependency (#15246) commit 5b3b3ab Author: Andrew Duthie <[email protected]> Date: Mon Apr 29 13:38:09 2019 -0400 Framework: Add REST API codeowners (#15215) commit 64c48fd Author: Ian Dunn <[email protected]> Date: Mon Apr 29 10:06:46 2019 -0700 Correct name of `date_i18n()` PHP function. (#15204) commit 8c28814 Author: Andrew Duthie <[email protected]> Date: Mon Apr 29 13:05:10 2019 -0400 Framework: Remove TESTS.md (#15217) commit 0f1fb57 Author: Emmanuel Hesry <[email protected]> Date: Mon Apr 29 17:34:22 2019 +0200 fix typo in withDispatch documentation (#15251) commit a5282ce Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 16:08:31 2019 +0300 Revert "Trivial change to trigger Travis" This reverts commit e22ffde. commit e22ffde Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 16:08:24 2019 +0300 Trivial change to trigger Travis commit 15a51f8 Author: Jorge Costa <[email protected]> Date: Fri Apr 19 15:33:15 2019 +0100 chore: Fix: Lint error that makes unit tests (and CI tests) fail. (#15073) commit 2b11074 Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 12:42:55 2019 +0300 Fix lint issues commit 9c298c8 Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 12:01:46 2019 +0300 Need to specify firedAfterTextChanged on all Aztec events commit 46e086d Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 03:59:59 2019 +0300 Force Aztec update if "Enter" fired before text change commit 724e295 Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 03:11:52 2019 +0300 Differentiate Android and iOS since assumptions diverged The iOS side still expects to just check against `this.lastContent` to force the change into Aztec. commit 52386fa Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 01:55:22 2019 +0300 Use a flag to signal Aztec-originated changes And assume that when that flag is false, component changes need to get sent/reflected down to Aztec. commit c7aa381 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 23 17:30:45 2019 +0300 Able to not lose content commit 60c75b0 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 23 10:09:23 2019 +0300 If text already changed, don't modify it
This PR does two things: - Fixes #15626 and #15680 by making the script synchronous. In #15200 we missed the fact that to fill tokens within the same file we need to execute docgen synchronously, otherwise, the last token process will overwrite the first. - Updates our espree dependency to 4.0.0. When error reporting was back, we uncovered that introducing short Fragment syntax in #15120 caused docgen to fail. The reason is that the espree version we used didn't support that. This fixes it by upgrading it to one that does. A couple of restrictions: - espree uses acorn-jsx to power JSX parsing. - [email protected] added support for JSX fragment short syntax (patched in 4.1.1). - [email protected] added [email protected]. We should use this at a minimum. - [email protected] added acorn@6 and acorn-jsx@5 for parsing. This caused an error I couldn't identify the source. - [email protected] removed support for the attachComment. We use this for collocating the JSDoc comment with the proper export statement. Without this, we can't migrate to [email protected].
* Better support for @wordpress/element Fragment JSX See WordPress/gutenberg#15120 * Upgrade dependencies * Add CHANGELOG entry * Add DefinePlugin FORCE_REDUCED_MOTION to webpack configs
* Better support for @wordpress/element Fragment JSX See WordPress/gutenberg#15120 * Upgrade dependencies * Add CHANGELOG entry * Add DefinePlugin FORCE_REDUCED_MOTION to webpack configs
Closes #12207
Description
As described in #12207, the Babel plugin package that adds imports for JSX now handles Fragments as well:
Replace
<Fragment />
usage with<>
and remove theFragment
imports where possible in the codebase. This may be better suited as a follow up PR.How has this been tested?
Includes additional tests.
Types of changes
New feature: Add Fragment handling to the Babel plugin import JSX pragma package as outlined in #12207.
<></>
will add aFragment
import ifscopeVariableFrag
is set and the import doesn't exist.Checklist: