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

Add jsx-runtime entry for the new automatic JSX transform #34221

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .npmpackagejsonlintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
packages/element/jsx-runtime/package.json
packages/element/jsx-dev-runtime/package.json
83 changes: 80 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@
"build:packages": "npm run build:package-types && node ./bin/packages/build.js",
"build:package-types": "node ./bin/packages/validate-typescript-version.js && tsc --build",
"build:plugin-zip": "./bin/build-plugin-zip.sh",
"build": "npm run build:packages && wp-scripts build",
"build": "cross-env NODE_ENV=production npm run build:packages && wp-scripts build",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we weren't doing this?

This let Babel knows that we're targeting production builds, and applies some prod-only enhancements. I guess it's because we didn't have any plugins that used this feature?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this was part of npm run build:packages internally no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm run build:packages is also used by npm run dev, so I think no.

Copy link
Member

@gziolo gziolo Aug 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use different environments in npm run build:packages:

  • main for build folders
  • module for build-module folders

I guess the current flow doesn't care whether it's a production or development env. We leave that part to webpack to decide.

As far as I can tell, the Babel preset for WordPress has a special handling only for the test env.

"changelog": "./bin/plugin/cli.js changelog",
"check-licenses": "concurrently \"wp-scripts check-licenses --prod --gpl2 --ignore=@react-native-community/cli,@react-native-community/cli-platform-ios\" \"wp-scripts check-licenses --dev\"",
"precheck-local-changes": "npm run docs:build",
Expand Down
4 changes: 4 additions & 0 deletions packages/babel-preset-default/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Breaking Changes

- Use `@babel/preset-react` with `@wordpress/element`'s automatic runtime to transform JSX ([#34221](https://github.com/WordPress/gutenberg/pull/34221)).

## 6.2.0 (2021-05-31)

### New Feature
Expand Down
25 changes: 9 additions & 16 deletions packages/babel-preset-default/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module.exports = ( api ) => {
);

const isTestEnv = api.env() === 'test';
const isDevEnv = api.env() === 'development';

api.caller( ( caller ) => {
if ( caller && isWPBuild( caller.name ) ) {
Expand Down Expand Up @@ -67,26 +68,18 @@ module.exports = ( api ) => {
return {
presets: [
getPresetEnv(),
require.resolve( '@babel/preset-typescript' ),
],
plugins: [
require.resolve( '@wordpress/warning/babel-plugin' ),
[
require.resolve( '@wordpress/babel-plugin-import-jsx-pragma' ),
{
scopeVariable: 'createElement',
scopeVariableFrag: 'Fragment',
source: '@wordpress/element',
isDefault: false,
},
],
[
require.resolve( '@babel/plugin-transform-react-jsx' ),
require.resolve( '@babel/preset-react' ),
{
pragma: 'createElement',
pragmaFrag: 'Fragment',
runtime: 'automatic',
importSource: '@wordpress/element',
development: isDevEnv,
},
],
require.resolve( '@babel/preset-typescript' ),
],
plugins: [
require.resolve( '@wordpress/warning/babel-plugin' ),
maybeGetPluginTransformRuntime(),
].filter( Boolean ),
};
Expand Down
4 changes: 1 addition & 3 deletions packages/babel-preset-default/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,12 @@
"main": "index.js",
"dependencies": {
"@babel/core": "^7.13.10",
"@babel/plugin-transform-react-jsx": "^7.12.7",
"@babel/plugin-transform-runtime": "^7.13.10",
"@babel/preset-env": "^7.13.10",
"@babel/preset-react": "^7.10.1",
"@babel/preset-typescript": "^7.13.0",
"@babel/runtime": "^7.13.10",
"@wordpress/babel-plugin-import-jsx-pragma": "file:../babel-plugin-import-jsx-pragma",
"@wordpress/browserslist-config": "file:../browserslist-config",
"@wordpress/element": "file:../element",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this as a dependency. Peer dependency, probably, but not a dependency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I don't remember why we set them as dependencies. Maybe for developers' convenience because they would need to add @wordpress/element as a dependency when using JSX anyway. I guess it's fine to remove, we just should mention it in the changelog.

"@wordpress/warning": "file:../warning",
"browserslist": "^4.16.6",
"core-js": "^3.12.1"
Expand Down
17 changes: 17 additions & 0 deletions packages/dependency-extraction-webpack-plugin/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ function defaultRequestToExternal( request ) {

case 'react-dom':
return 'ReactDOM';

case 'object-assign':
return [ 'Object', 'assign' ];

case '@wordpress/element/jsx-runtime':
return [ 'wp', 'jsxRuntime' ];
case '@wordpress/element/jsx-dev-runtime':
return [ 'wp', 'jsxDevRuntime' ];
}

if ( BUNDLED_PACKAGES.includes( request ) ) {
Expand Down Expand Up @@ -64,6 +72,15 @@ function defaultRequestToHandle( request ) {

case 'lodash-es':
return 'lodash';

// Ignore object-assign ponyfill, which is used by jsx-(dev)-runtime.
gziolo marked this conversation as resolved.
Show resolved Hide resolved
case 'object-assign':
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
return 'wp-polyfill';

case '@wordpress/element/jsx-runtime':
return 'wp-jsx-runtime';
case '@wordpress/element/jsx-dev-runtime':
return 'wp-jsx-dev-runtime';
}

if ( request.startsWith( WORDPRESS_NAMESPACE ) ) {
Expand Down
4 changes: 4 additions & 0 deletions packages/element/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### New Features

- Added `jsx-runtime` and `jsx-dev-runtime` modules to support the new automatic JSX transform ([#34221](https://github.com/WordPress/gutenberg/pull/34221)).

## 4.0.0 (2021-07-29)

### Breaking Change
Expand Down
1 change: 1 addition & 0 deletions packages/element/jsx-dev-runtime/jsx-dev-runtime.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'react/jsx-dev-runtime';
7 changes: 7 additions & 0 deletions packages/element/jsx-dev-runtime/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved
"main": "../build/jsx-dev-runtime.js",
"module": "../build-module/jsx-dev-runtime.js",
"react-native": "../src/jsx-dev-runtime.js",
"types": "./jsx-dev-runtime.d.ts",
"sideEffects": false
}
1 change: 1 addition & 0 deletions packages/element/jsx-runtime/jsx-runtime.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'react/jsx-runtime';
7 changes: 7 additions & 0 deletions packages/element/jsx-runtime/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"main": "../build/jsx-runtime.js",
"module": "../build-module/jsx-runtime.js",
"react-native": "../src/jsx-runtime.js",
"types": "./jsx-runtime.d.ts",
"sideEffects": false
}
1 change: 1 addition & 0 deletions packages/element/src/jsx-dev-runtime.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from 'react/jsx-dev-runtime';
1 change: 1 addition & 0 deletions packages/element/src/jsx-runtime.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from 'react/jsx-runtime';
2 changes: 2 additions & 0 deletions test/unit/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ module.exports = {
[ `@wordpress\\/(${ transpiledPackageNames.join(
'|'
) })$` ]: 'packages/$1/src',
'@wordpress\\/element\\/(jsx\\-runtime|jsx\\-dev\\-runtime)$':
'packages/element/src/$1',
},
preset: '@wordpress/jest-preset-default',
setupFiles: [
Expand Down
40 changes: 29 additions & 11 deletions tools/webpack/packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,39 @@ const exportDefaultPackages = [
module.exports = {
...baseConfig,
name: 'packages',
entry: gutenbergPackages.reduce( ( memo, packageName ) => {
return {
...memo,
[ packageName ]: {
import: `./packages/${ packageName }`,
entry: gutenbergPackages.reduce(
( memo, packageName ) => {
return {
...memo,
[ packageName ]: {
import: `./packages/${ packageName }`,
library: {
name: [ 'wp', camelCaseDash( packageName ) ],
type: 'window',
export: exportDefaultPackages.includes( packageName )
? 'default'
: undefined,
},
},
};
},
{
'jsx-runtime': {
import: './packages/element/jsx-runtime',
library: {
name: [ 'wp', 'jsxRuntime' ],
type: 'window',
},
},
'jsx-dev-runtime': {
import: './packages/element/jsx-dev-runtime',
library: {
name: [ 'wp', camelCaseDash( packageName ) ],
name: [ 'wp', 'jsxDevRuntime' ],
type: 'window',
export: exportDefaultPackages.includes( packageName )
? 'default'
: undefined,
},
},
};
}, {} ),
}
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes here need to be backported to Core's webpack config later as well.

output: {
devtoolNamespace: 'wp',
filename: './build/[name]/index.min.js',
Expand Down
3 changes: 2 additions & 1 deletion tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"allowJs": true,
"checkJs": true,
"allowSyntheticDefaultImports": true,
"jsx": "preserve",
"jsx": "react-jsx",
"jsxImportSource": "@wordpress/element",
"target": "esnext",
"module": "esnext",
"lib": [ "dom", "esnext" ],
Expand Down