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

JSX: Use <> Fragments with transforms #15261

Merged
merged 2 commits into from May 7, 2019

Conversation

@sirreal
Copy link
Member

commented Apr 30, 2019

Description

#15120 adds Fragment handling to @wordpress/babel-plugin-import-jsx-pragma and prepares @wordpress/babel-preset-default for the WordPress environment.

Leverage the <></> automatic Fragment import provided by the babel transform included in the preset. Follow-up to #15120.

How has this been tested?

Inspect transformed code to ensure imports and JSX transforms are working correctly.
Smoke test code using <></>.

Types of changes

Janitorial.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@sirreal sirreal self-assigned this Apr 30, 2019

@sirreal sirreal changed the title Update/use empty jsx fragments JSX: Use <> Fragments with transforms Apr 30, 2019

@sirreal sirreal referenced this pull request Apr 30, 2019

Merged

Babel plugin JSX: Implement Fragment handling #15120

5 of 5 tasks complete

@sirreal sirreal force-pushed the update/use-empty-jsx-fragments branch 2 times, most recently from 17f8101 to c34aed0 Apr 30, 2019

@sirreal sirreal marked this pull request as ready for review May 4, 2019

@gziolo

This comment has been minimized.

Copy link
Member

commented May 6, 2019

I like how it simplifies the code by removing the need to import Fragment explicitly 👍

I'm wondering whether it should be enforced to use <> in Gutenberg from now on with ESLint rule?

Another question is, how we can communicate in docs that both createElement and Fragment are handled behind the scenes when you use the recommended Babel plugin. /cc @mkaz and @nosolosw

@sirreal

This comment has been minimized.

Copy link
Member Author

commented May 6, 2019

I'm wondering whether it should be enforced to use <> in Gutenberg from now on with ESLint rule?

I do think this syntax should be preferred, however there are still places where <Fragment key={ key }> is necessarily used because <> does not support keys. Any rule would need to be smart enough to discern. In this PR I left files including Fragments with and without keys to use the full <Fragment> in all cases.

@aduth

aduth approved these changes May 6, 2019

Copy link
Member

left a comment

There's a merge conflict to be addressed. It otherwise appears in good order. The specific syntax is a bit off-putting to me (vs. the "explicit" Fragment), but I sense it's a matter of becoming accustomed to, and can be reinforced as a pattern increasingly common to any React project.

sirreal added some commits Apr 24, 2019

@sirreal sirreal force-pushed the update/use-empty-jsx-fragments branch from c34aed0 to da935a6 May 7, 2019

@sirreal sirreal merged commit 39f568f into master May 7, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@sirreal sirreal deleted the update/use-empty-jsx-fragments branch May 7, 2019

@gziolo

This comment has been minimized.

Copy link
Member

commented May 7, 2019

The specific syntax is a bit off-putting to me (vs. the "explicit" Fragment), but I sense it's a matter of becoming accustomed to, and can be reinforced as a pattern increasingly common to any React project.

I share the same sentiment, in general, it should simplify everything :)

@youknowriad youknowriad added this to the 5.7 (Gutenberg) milestone May 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.