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.

Sign up for GitHub

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

Jump to bottom

Babel Preset Default: Add polyfill for WordPress built from core-js #31279

Merged
merged 2 commits into from
May 22, 2021
Merged

Babel Preset Default: Add polyfill for WordPress built from core-js #31279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41699d6
Babel Preset Default: Add polyfill for WordPress built from core-js
gziolo Apr 28, 2021
9ef7611
Address feedback from code review
gziolo May 22, 2021
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
1 change: 1 addition & 0 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const developmentFiles = [
'**/benchmark/**/*.js',
'**/@(__mocks__|__tests__|test)/**/*.js',
'**/@(storybook|stories)/**/*.js',
'packages/babel-preset-default/bin/**/*.js',
];

// All files from packages that have types provided with TypeScript.
Expand Down
359 changes: 109 additions & 250 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@
"commander": "4.1.0",
"concurrently": "3.5.0",
"copy-webpack-plugin": "5.1.2",
"core-js-builder": "3.11.0",
"cross-env": "3.2.4",
"css-loader": "5.1.3",
"cssnano": "4.1.10",
Expand Down Expand Up @@ -205,6 +206,7 @@
"style-loader": "2.0.0",
"terser-webpack-plugin": "3.0.3",
"typescript": "4.1.3",
"uglify-js": "3.13.7",
"uuid": "8.3.0",
"wd": "1.12.1",
"webpack": "4.46.0",
Expand Down
4 changes: 4 additions & 0 deletions packages/babel-preset-default/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### New Feature

- New `build/polyfill.js` (minified version – `build/polyfill.min.js`) file is available that polyfills ECMAScript features missing in the [browsers supported](https://make.wordpress.org/core/handbook/best-practices/browser-support/) by the WordPress project. It's a drop-in replacement for the deprecated `@babel/polyfill` package ([#31279](https://github.com/WordPress/gutenberg/pull/31279)).

## 6.1.0 (2021-05-20)

### Bug Fixes
Expand Down
10 changes: 10 additions & 0 deletions packages/babel-preset-default/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,14 @@ For example, if you'd like to use a new language feature proposal which has not
}
```

### Polyfill

There is a complementary `build/polyfill.js` (minified version – `build/polyfill.min.js`) file available that polyfills ECMAScript features missing in the [browsers supported](https://make.wordpress.org/core/handbook/best-practices/browser-support/) by the WordPress project ([#31279](https://github.com/WordPress/gutenberg/pull/31279)). It's a drop-in replacement for the deprecated `@babel/polyfill` package, and it's also based on [`core-js`](https://github.com/zloirock/core-js) project.

This needs to be included before all your compiled Babel code. You can either prepend it to your compiled code or include it in a `<script>` before it.

#### TC39 Proposals

If you need to use a proposal that is not Stage 4, this polyfill will not automatically import those for you. You will have to import those from another polyfill like [`core-js`](https://github.com/zloirock/core-js) individually.

<br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p>
33 changes: 33 additions & 0 deletions packages/babel-preset-default/bin/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/usr/bin/env node

Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

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

@gziolo: As much as I love shebangs, AFAIK they just don't work well or at all with Windows. This just emerged in #32329. I'm wondering if we should/could forbid it with our linter — wdyt?

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 copied it over from other scripts. What would be the alternative? If it causes issues we should teach ESLint to prevent issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

The alternative would be to just not have any shebang, meaning that scripts would just have code:

1. /**
2.  * External dependencies
3.  */
4. const builder = ...;

This would make it impossible for anyone in a UNIX system to invoke ./index.js (they would have to explicitly call the interpreter with node index.js), thus eliminating the possibility of merging something that would break in Windows.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, we can live with it 👍🏻

Copy link
Member Author

Choose a reason for hiding this comment

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

Filed: #32504.

/**
* External dependencies
*/
const builder = require( 'core-js-builder' );
const { minify } = require( 'uglify-js' );
const { writeFile } = require( 'fs' ).promises;

builder( {
modules: [ 'es', 'web' ],
Copy link
Member Author

Choose a reason for hiding this comment

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

It's based on:

https://github.com/zloirock/core-js/blob/67ad54d1f1c455c300e4757e72039d8b586b743c/scripts/generate-indexes.js#L26

// core-js is extremely conservative in which polyfills to include.
// Knowing about tiny browser implementation bugs that anyone rarely cares about,
// we prevent some features from having the full polyfill included.
Copy link
Member

Choose a reason for hiding this comment

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

I don't really understand the comment. What does core-js being conservative have to do with use excluding more things? And what tiny browser implementation bugs? To me the latter makes it sound like it reimplements everything?

// @see https://github.com/WordPress/gutenberg/pull/31279
exclude: [ 'es.promise' ],
gziolo marked this conversation as resolved.
Show resolved Hide resolved
targets: require( '@wordpress/browserslist-config' ),
gziolo marked this conversation as resolved.
Show resolved Hide resolved
filename: './build/polyfill.js',
} )
.then( async ( code ) => {
const output = minify( code, {
output: {
comments: ( node, comment ) =>
comment.value.indexOf( 'License' ) >= 0,
},
} );
await writeFile( './build/polyfill.min.js', output.code );
} )
.catch( ( error ) => {
// eslint-disable-next-line no-console
console.log( error );
process.exit( 1 );
} );
6 changes: 5 additions & 1 deletion packages/babel-preset-default/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"node": ">=12"
},
"files": [
"build",
"index.js"
],
"main": "index.js",
Expand All @@ -39,9 +40,12 @@
"@wordpress/element": "file:../element",
"@wordpress/warning": "file:../warning",
"browserslist": "^4.16.6",
"core-js": "^3.6.4"
"core-js": "^3.12.1"
gziolo marked this conversation as resolved.
Show resolved Hide resolved
},
"publishConfig": {
"access": "public"
},
"scripts": {
"build": "./bin/index.js"
}
}
2 changes: 1 addition & 1 deletion packages/react-native-editor/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"@wordpress/i18n": "file:../i18n",
"@wordpress/react-native-aztec": "file:../react-native-aztec",
"@wordpress/react-native-bridge": "file:../react-native-bridge",
"core-js": "^3.8.3",
"core-js": "^3.12.1",
"fast-average-color": "^4.3.0",
"jed": "^1.1.1",
"jsc-android": "^241213.1.0",
Expand Down