From d715e93b9802278a8c18cbca2132e9478a588206 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Tue, 30 Apr 2019 16:29:09 +0200 Subject: [PATCH] Babel plugin JSX: Implement Fragment handling (#15120) Add imports for `<>` JSX Fragments. --- .../CHANGELOG.md | 8 +- .../babel-plugin-import-jsx-pragma/README.md | 12 ++- .../babel-plugin-import-jsx-pragma/index.js | 80 ++++++++++----- .../test/index.js | 99 ++++++++++++++++--- packages/babel-preset-default/CHANGELOG.md | 6 ++ packages/babel-preset-default/index.js | 2 + 6 files changed, 163 insertions(+), 44 deletions(-) diff --git a/packages/babel-plugin-import-jsx-pragma/CHANGELOG.md b/packages/babel-plugin-import-jsx-pragma/CHANGELOG.md index 69c48331c9b90a..27860040e61090 100644 --- a/packages/babel-plugin-import-jsx-pragma/CHANGELOG.md +++ b/packages/babel-plugin-import-jsx-pragma/CHANGELOG.md @@ -1,3 +1,9 @@ +## 2.1.0 (unreleased) + +### New Feature + +- Add Fragment import handling ([#15120](https://github.com/WordPress/gutenberg/pull/15120)). + ## 2.0.0 (2019-03-06) ### Breaking Change @@ -6,7 +12,7 @@ ### Enhancement -- Plugin skips now adding import JSX pragma when the scope variable is defined for all JSX elements ([#13809](https://github.com/WordPress/gutenberg/pull/13809)). +- Plugin skips now adding import JSX pragma when the scope variable is defined for all JSX elements ([#13809](https://github.com/WordPress/gutenberg/pull/13809)). ## 1.1.0 (2018-09-05) diff --git a/packages/babel-plugin-import-jsx-pragma/README.md b/packages/babel-plugin-import-jsx-pragma/README.md index aa43bc1285dcc7..2459cea08be451 100644 --- a/packages/babel-plugin-import-jsx-pragma/README.md +++ b/packages/babel-plugin-import-jsx-pragma/README.md @@ -44,11 +44,13 @@ module.exports = { plugins: [ [ '@wordpress/babel-plugin-import-jsx-pragma', { scopeVariable: 'createElement', + scopeVariableFrag: 'Fragment', source: '@wordpress/element', isDefault: false, } ], [ '@babel/transform-react-jsx', { pragma: 'createElement', + pragmaFrag: 'Fragment', } ], ], }; @@ -60,6 +62,13 @@ _Type:_ String Name of variable required to be in scope for use by the JSX pragma. For the default pragma of React.createElement, the React variable must be within scope. +### `scopeVariableFrag` + +_Type:_ String + +Name of variable required to be in scope for `<>` `Fragment` JSX. Named `` elements +expect Fragment to be in scope and will not add the import. + ### `source` _Type:_ String @@ -70,6 +79,7 @@ The module from which the scope variable is to be imported when missing. _Type:_ Boolean -Whether the scopeVariable is the default import of the source module. +Whether the scopeVariable is the default import of the source module. Note that this has no impact +on `scopeVariableFrag`.

Code is Poetry.

diff --git a/packages/babel-plugin-import-jsx-pragma/index.js b/packages/babel-plugin-import-jsx-pragma/index.js index 7953640c484ab3..273df752611c15 100644 --- a/packages/babel-plugin-import-jsx-pragma/index.js +++ b/packages/babel-plugin-import-jsx-pragma/index.js @@ -1,17 +1,20 @@ /** * Default options for the plugin. * - * @property {string} scopeVariable Name of variable required to be in scope - * for use by the JSX pragma. For the default - * pragma of React.createElement, the React - * variable must be within scope. - * @property {string} source The module from which the scope variable - * is to be imported when missing. - * @property {boolean} isDefault Whether the scopeVariable is the default - * import of the source module. + * @property {string} scopeVariable Name of variable required to be in scope + * for use by the JSX pragma. For the default + * pragma of React.createElement, the React + * variable must be within scope. + * @property {string} scopeVariableFrag Name of variable required to be in scope + * for use by the Fragment pragma. + * @property {string} source The module from which the scope variable + * is to be imported when missing. + * @property {boolean} isDefault Whether the scopeVariable is the default + * import of the source module. */ const DEFAULT_OPTIONS = { scopeVariable: 'React', + scopeVariableFrag: null, source: 'react', isDefault: true, }; @@ -39,7 +42,7 @@ module.exports = function( babel ) { return { visitor: { - JSXElement( path, state ) { + JSX( path, state ) { if ( state.hasUndeclaredScopeVariable ) { return; } @@ -47,32 +50,55 @@ module.exports = function( babel ) { const { scopeVariable } = getOptions( state ); state.hasUndeclaredScopeVariable = ! path.scope.hasBinding( scopeVariable ); }, + JSXFragment( path, state ) { + if ( state.hasUndeclaredScopeVariableFrag ) { + return; + } + + const { scopeVariableFrag } = getOptions( state ); + if ( scopeVariableFrag === null ) { + return; + } + + state.hasUndeclaredScopeVariableFrag = ! path.scope.hasBinding( scopeVariableFrag ); + }, Program: { exit( path, state ) { - if ( ! state.hasUndeclaredScopeVariable ) { - return; - } + const { scopeVariable, scopeVariableFrag, source, isDefault } = getOptions( state ); - const { scopeVariable, source, isDefault } = getOptions( state ); + let scopeVariableSpecifier; + let scopeVariableFragSpecifier; - let specifier; - if ( isDefault ) { - specifier = t.importDefaultSpecifier( - t.identifier( scopeVariable ) - ); - } else { - specifier = t.importSpecifier( - t.identifier( scopeVariable ), - t.identifier( scopeVariable ) + if ( state.hasUndeclaredScopeVariable ) { + if ( isDefault ) { + scopeVariableSpecifier = t.importDefaultSpecifier( t.identifier( scopeVariable ) ); + } else { + scopeVariableSpecifier = t.importSpecifier( + t.identifier( scopeVariable ), + t.identifier( scopeVariable ) + ); + } + } + + if ( state.hasUndeclaredScopeVariableFrag ) { + scopeVariableFragSpecifier = t.importSpecifier( + t.identifier( scopeVariableFrag ), + t.identifier( scopeVariableFrag ) ); } - const importDeclaration = t.importDeclaration( - [ specifier ], - t.stringLiteral( source ) - ); + const importDeclarationSpecifiers = [ + scopeVariableSpecifier, + scopeVariableFragSpecifier, + ].filter( Boolean ); + if ( importDeclarationSpecifiers.length ) { + const importDeclaration = t.importDeclaration( + importDeclarationSpecifiers, + t.stringLiteral( source ) + ); - path.unshiftContainer( 'body', importDeclaration ); + path.unshiftContainer( 'body', importDeclaration ); + } }, }, }, diff --git a/packages/babel-plugin-import-jsx-pragma/test/index.js b/packages/babel-plugin-import-jsx-pragma/test/index.js index a10207a5d55634..c3e098476db3d2 100644 --- a/packages/babel-plugin-import-jsx-pragma/test/index.js +++ b/packages/babel-plugin-import-jsx-pragma/test/index.js @@ -9,18 +9,6 @@ import { transformSync } from '@babel/core'; import plugin from '../'; describe( 'babel-plugin-import-jsx-pragma', () => { - function getTransformedCode( source, options = {} ) { - const { code } = transformSync( source, { - configFile: false, - plugins: [ - [ plugin, options ], - '@babel/plugin-syntax-jsx', - ], - } ); - - return code; - } - it( 'does nothing if there is no jsx', () => { const original = 'let foo;'; const string = getTransformedCode( original ); @@ -49,6 +37,13 @@ describe( 'babel-plugin-import-jsx-pragma', () => { expect( string ).toBe( 'import React from "react";\n' + original ); } ); + 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 = <>;' ); + } ); + it( 'allows options customization', () => { const original = 'let foo = ;'; const string = getTransformedCode( original, { @@ -61,7 +56,8 @@ describe( 'babel-plugin-import-jsx-pragma', () => { } ); it( 'adds import for scope variable even when defined inside the local scope', () => { - const original = 'let foo = ;\n\nfunction local() {\n const createElement = wp.element.createElement;\n}'; + const original = + 'let foo = ;\n\nfunction local() {\n const createElement = wp.element.createElement;\n}'; const string = getTransformedCode( original, { scopeVariable: 'createElement', source: '@wordpress/element', @@ -72,20 +68,93 @@ describe( 'babel-plugin-import-jsx-pragma', () => { } ); it( 'does nothing if the outer scope variable is already defined when using custom options', () => { - const original = 'const {\n createElement\n} = wp.element;\nlet foo = ;'; + const original = + 'const {\n createElement,\n Fragment\n} = wp.element;\nlet foo = <>;'; const string = getTransformedCode( original, { scopeVariable: 'createElement', + scopeVariableFrag: 'Fragment', + source: '@wordpress/element', + isDefault: false, } ); expect( string ).toBe( original ); } ); + it( 'adds only Fragment when required', () => { + const original = 'const {\n createElement\n} = wp.element;\nlet foo = <>;'; + const string = getTransformedCode( original, { + scopeVariable: 'createElement', + scopeVariableFrag: 'Fragment', + source: '@wordpress/element', + isDefault: false, + } ); + + expect( string ).toBe( + 'import { Fragment } from "@wordpress/element";\nconst {\n createElement\n} = wp.element;\nlet foo = <>;' + ); + } ); + + it( 'adds only createElement when required', () => { + const original = 'const {\n Fragment\n} = wp.element;\nlet foo = <>;'; + const string = getTransformedCode( original, { + scopeVariable: 'createElement', + scopeVariableFrag: 'Fragment', + source: '@wordpress/element', + isDefault: false, + } ); + + expect( string ).toBe( + 'import { createElement } from "@wordpress/element";\nconst {\n Fragment\n} = wp.element;\nlet foo = <>;' + ); + } ); + it( 'does nothing if the inner scope variable is already defined when using custom options', () => { - const original = '(function () {\n const {\n createElement\n } = wp.element;\n let foo = ;\n})();'; + const original = + '(function () {\n const {\n createElement\n } = wp.element;\n let foo = ;\n})();'; const string = getTransformedCode( original, { scopeVariable: 'createElement', + scopeVariableFrag: 'Fragment', + source: '@wordpress/element', + isDefault: false, } ); expect( string ).toBe( original ); } ); + + it( 'adds Fragment as for <>', () => { + const original = 'let foo = <>;'; + const string = getTransformedCode( original, { + scopeVariable: 'createElement', + scopeVariableFrag: 'Fragment', + source: '@wordpress/element', + isDefault: false, + } ); + + expect( string ).toBe( + 'import { createElement, Fragment } from "@wordpress/element";\nlet foo = <>;' + ); + } ); + + it( 'does not add Fragment import for ', () => { + const original = 'let foo = ;'; + const string = getTransformedCode( original, { + scopeVariable: 'createElement', + scopeVariableFrag: 'Fragment', + source: '@wordpress/element', + isDefault: false, + } ); + + expect( string ).toBe( + 'import { createElement } from "@wordpress/element";\nlet foo = ;' + ); + } ); } ); + +function getTransformedCode( source, options = {} ) { + const { code } = transformSync( source, { + configFile: false, + plugins: [ [ plugin, options ], '@babel/plugin-syntax-jsx' ], + } ); + + return code; +} diff --git a/packages/babel-preset-default/CHANGELOG.md b/packages/babel-preset-default/CHANGELOG.md index ab2a808cdfee40..dde12959c59bc6 100644 --- a/packages/babel-preset-default/CHANGELOG.md +++ b/packages/babel-preset-default/CHANGELOG.md @@ -1,3 +1,9 @@ +## 4.1.0 (unreleased) + +### New Feature + +- Handle `<>` JSX Fragments with `@wordpress/element` `Fragment` ([#15120](https://github.com/WordPress/gutenberg/pull/15120)). + ## 4.0.0 (2019-03-06) ### Breaking Change diff --git a/packages/babel-preset-default/index.js b/packages/babel-preset-default/index.js index 5c7f22d98439a8..d0df58873572c2 100644 --- a/packages/babel-preset-default/index.js +++ b/packages/babel-preset-default/index.js @@ -58,12 +58,14 @@ module.exports = function( api ) { require.resolve( '@wordpress/babel-plugin-import-jsx-pragma' ), { scopeVariable: 'createElement', + scopeVariableFrag: 'Fragment', source: '@wordpress/element', isDefault: false, }, ], [ require.resolve( '@babel/plugin-transform-react-jsx' ), { pragma: 'createElement', + pragmaFrag: 'Fragment', } ], require.resolve( '@babel/plugin-proposal-async-generator-functions' ), maybeGetPluginTransformRuntime(),