From 6f5fa88865a44cfd219665939e65a9db7da3aeb3 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Tue, 23 Apr 2019 11:21:05 +0200 Subject: [PATCH 1/7] Implement Fragment handling --- .../babel-plugin-import-jsx-pragma/index.js | 90 ++++++++++++----- .../test/index.js | 99 ++++++++++++++++--- 2 files changed, 147 insertions(+), 42 deletions(-) diff --git a/packages/babel-plugin-import-jsx-pragma/index.js b/packages/babel-plugin-import-jsx-pragma/index.js index 7953640c484ab3..ad774b218087e1 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, }; @@ -32,6 +35,11 @@ module.exports = function( babel ) { function getOptions( state ) { if ( ! state._options ) { state._options = Object.assign( {}, DEFAULT_OPTIONS, state.opts ); + if ( state._options.isDefault && state._options.scopeVariableFrag ) { + // eslint-disable-next-line no-console + console.warn( 'scopeVariableFrag is only available when isDefault is false' ); + state._options.scopeVariableFrag = null; + } } return state._options; @@ -39,7 +47,7 @@ module.exports = function( babel ) { return { visitor: { - JSXElement( path, state ) { + JSX( path, state ) { if ( state.hasUndeclaredScopeVariable ) { return; } @@ -47,32 +55,60 @@ module.exports = function( babel ) { const { scopeVariable } = getOptions( state ); state.hasUndeclaredScopeVariable = ! path.scope.hasBinding( scopeVariable ); }, + 'JSXElement|JSXFragment'( path, state ) { + if ( state.hasUndeclaredScopeVariableFrag ) { + return; + } + + const { scopeVariableFrag } = getOptions( state ); + if ( scopeVariableFrag === null ) { + return; + } + + if ( + path.type === 'JSXFragment' || + ( path.type === 'JSXElement' && path.node.openingElement.name.name === 'Fragment' ) + ) { + 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..fbd21615abc301 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( 'adds Fragment import for Fragment', () => { + 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 = ;' + ); + } ); } ); + +function getTransformedCode( source, options = {} ) { + const { code } = transformSync( source, { + configFile: false, + plugins: [ [ plugin, options ], '@babel/plugin-syntax-jsx' ], + } ); + + return code; +} From 13a2d6fa8d09703e93d77b2b4dba1f0571095123 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Tue, 23 Apr 2019 14:32:48 +0200 Subject: [PATCH 2/7] Document scopeVariableFrag --- packages/babel-plugin-import-jsx-pragma/README.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/babel-plugin-import-jsx-pragma/README.md b/packages/babel-plugin-import-jsx-pragma/README.md index aa43bc1285dcc7..ca6d7dc2c27631 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,12 @@ _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` (`<>` or ``) JSX. + ### `source` _Type:_ String @@ -70,6 +78,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.

From 635798d589b24503955e84bede53756579e24849 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Tue, 23 Apr 2019 14:35:05 +0200 Subject: [PATCH 3/7] Add changelog entry --- packages/babel-plugin-import-jsx-pragma/CHANGELOG.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/babel-plugin-import-jsx-pragma/CHANGELOG.md b/packages/babel-plugin-import-jsx-pragma/CHANGELOG.md index 69c48331c9b90a..3fca88ab062cba 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) + +### Enhancement + +- 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) From 1ec0ebf76337caa9f435d4d0bc2f2737e6aa6ca0 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Tue, 23 Apr 2019 14:44:02 +0200 Subject: [PATCH 4/7] Drop handling of named --- packages/babel-plugin-import-jsx-pragma/README.md | 3 ++- packages/babel-plugin-import-jsx-pragma/index.js | 7 ++----- packages/babel-plugin-import-jsx-pragma/test/index.js | 4 ++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/babel-plugin-import-jsx-pragma/README.md b/packages/babel-plugin-import-jsx-pragma/README.md index ca6d7dc2c27631..2459cea08be451 100644 --- a/packages/babel-plugin-import-jsx-pragma/README.md +++ b/packages/babel-plugin-import-jsx-pragma/README.md @@ -66,7 +66,8 @@ Name of variable required to be in scope for use by the JSX pragma. For the defa _Type:_ String -Name of variable required to be in scope for `Fragment` (`<>` or ``) JSX. +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` diff --git a/packages/babel-plugin-import-jsx-pragma/index.js b/packages/babel-plugin-import-jsx-pragma/index.js index ad774b218087e1..a39d4320ebbc49 100644 --- a/packages/babel-plugin-import-jsx-pragma/index.js +++ b/packages/babel-plugin-import-jsx-pragma/index.js @@ -55,7 +55,7 @@ module.exports = function( babel ) { const { scopeVariable } = getOptions( state ); state.hasUndeclaredScopeVariable = ! path.scope.hasBinding( scopeVariable ); }, - 'JSXElement|JSXFragment'( path, state ) { + JSXFragment( path, state ) { if ( state.hasUndeclaredScopeVariableFrag ) { return; } @@ -65,10 +65,7 @@ module.exports = function( babel ) { return; } - if ( - path.type === 'JSXFragment' || - ( path.type === 'JSXElement' && path.node.openingElement.name.name === 'Fragment' ) - ) { + if ( path.type === 'JSXFragment' ) { state.hasUndeclaredScopeVariableFrag = ! path.scope.hasBinding( scopeVariableFrag ); } }, diff --git a/packages/babel-plugin-import-jsx-pragma/test/index.js b/packages/babel-plugin-import-jsx-pragma/test/index.js index fbd21615abc301..c3e098476db3d2 100644 --- a/packages/babel-plugin-import-jsx-pragma/test/index.js +++ b/packages/babel-plugin-import-jsx-pragma/test/index.js @@ -135,7 +135,7 @@ describe( 'babel-plugin-import-jsx-pragma', () => { ); } ); - it( 'adds Fragment import for Fragment', () => { + it( 'does not add Fragment import for ', () => { const original = 'let foo = ;'; const string = getTransformedCode( original, { scopeVariable: 'createElement', @@ -145,7 +145,7 @@ describe( 'babel-plugin-import-jsx-pragma', () => { } ); expect( string ).toBe( - 'import { createElement, Fragment } from "@wordpress/element";\nlet foo = ;' + 'import { createElement } from "@wordpress/element";\nlet foo = ;' ); } ); } ); From c6ae6b4722535e4284b029bbc022b121bcfe24c4 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Wed, 24 Apr 2019 12:25:41 +0200 Subject: [PATCH 5/7] Update babel preset to handle JSX Fragments --- packages/babel-plugin-import-jsx-pragma/CHANGELOG.md | 2 +- packages/babel-preset-default/CHANGELOG.md | 6 ++++++ packages/babel-preset-default/index.js | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/babel-plugin-import-jsx-pragma/CHANGELOG.md b/packages/babel-plugin-import-jsx-pragma/CHANGELOG.md index 3fca88ab062cba..27860040e61090 100644 --- a/packages/babel-plugin-import-jsx-pragma/CHANGELOG.md +++ b/packages/babel-plugin-import-jsx-pragma/CHANGELOG.md @@ -1,6 +1,6 @@ ## 2.1.0 (unreleased) -### Enhancement +### New Feature - Add Fragment import handling ([#15120](https://github.com/WordPress/gutenberg/pull/15120)). 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(), From 36faddfa4d11dcf5b5438e50fe63d49d40f63b56 Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Tue, 30 Apr 2019 09:13:19 +0200 Subject: [PATCH 6/7] Remove bad warn on isDefault with scopeVariableFrag Via https://github.com/WordPress/gutenberg/pull/15120/files#r279531622 --- packages/babel-plugin-import-jsx-pragma/index.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/babel-plugin-import-jsx-pragma/index.js b/packages/babel-plugin-import-jsx-pragma/index.js index a39d4320ebbc49..767b39c17de3a0 100644 --- a/packages/babel-plugin-import-jsx-pragma/index.js +++ b/packages/babel-plugin-import-jsx-pragma/index.js @@ -35,11 +35,6 @@ module.exports = function( babel ) { function getOptions( state ) { if ( ! state._options ) { state._options = Object.assign( {}, DEFAULT_OPTIONS, state.opts ); - if ( state._options.isDefault && state._options.scopeVariableFrag ) { - // eslint-disable-next-line no-console - console.warn( 'scopeVariableFrag is only available when isDefault is false' ); - state._options.scopeVariableFrag = null; - } } return state._options; From 9e13d18d88f1cd01692db683b63f1015fade5d5f Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Tue, 30 Apr 2019 14:56:21 +0200 Subject: [PATCH 7/7] Drop redundant conditional --- packages/babel-plugin-import-jsx-pragma/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/babel-plugin-import-jsx-pragma/index.js b/packages/babel-plugin-import-jsx-pragma/index.js index 767b39c17de3a0..273df752611c15 100644 --- a/packages/babel-plugin-import-jsx-pragma/index.js +++ b/packages/babel-plugin-import-jsx-pragma/index.js @@ -60,9 +60,7 @@ module.exports = function( babel ) { return; } - if ( path.type === 'JSXFragment' ) { - state.hasUndeclaredScopeVariableFrag = ! path.scope.hasBinding( scopeVariableFrag ); - } + state.hasUndeclaredScopeVariableFrag = ! path.scope.hasBinding( scopeVariableFrag ); }, Program: { exit( path, state ) {