From 234e5b671f4ce5abf63453721841e480fa0f5ee0 Mon Sep 17 00:00:00 2001 From: Haz Date: Tue, 24 Dec 2019 00:15:42 -0300 Subject: [PATCH 01/18] Add new warning package --- bin/api-docs/packages.js | 1 + docs/manifest-devhub.json | 6 ++++ packages/warning/.npmrc | 1 + packages/warning/CHANGELOG.md | 0 packages/warning/README.md | 44 ++++++++++++++++++++++++++++++ packages/warning/package.json | 27 ++++++++++++++++++ packages/warning/src/index.js | 37 +++++++++++++++++++++++++ packages/warning/src/test/index.js | 30 ++++++++++++++++++++ 8 files changed, 146 insertions(+) create mode 100644 packages/warning/.npmrc create mode 100644 packages/warning/CHANGELOG.md create mode 100644 packages/warning/README.md create mode 100644 packages/warning/package.json create mode 100644 packages/warning/src/index.js create mode 100644 packages/warning/src/test/index.js diff --git a/bin/api-docs/packages.js b/bin/api-docs/packages.js index ff96517799bf9f..dcc583fe65d0e2 100644 --- a/bin/api-docs/packages.js +++ b/bin/api-docs/packages.js @@ -32,6 +32,7 @@ const packages = [ 'shortcode', 'url', 'viewport', + 'warning', 'wordcount', ]; diff --git a/docs/manifest-devhub.json b/docs/manifest-devhub.json index 4aa9e183b6d36d..f58a2257183d53 100644 --- a/docs/manifest-devhub.json +++ b/docs/manifest-devhub.json @@ -1475,6 +1475,12 @@ "markdown_source": "../packages/viewport/README.md", "parent": "packages" }, + { + "title": "@wordpress/warning", + "slug": "packages-warning", + "markdown_source": "../packages/warning/README.md", + "parent": "packages" + }, { "title": "@wordpress/wordcount", "slug": "packages-wordcount", diff --git a/packages/warning/.npmrc b/packages/warning/.npmrc new file mode 100644 index 00000000000000..43c97e719a5a82 --- /dev/null +++ b/packages/warning/.npmrc @@ -0,0 +1 @@ +package-lock=false diff --git a/packages/warning/CHANGELOG.md b/packages/warning/CHANGELOG.md new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/packages/warning/README.md b/packages/warning/README.md new file mode 100644 index 00000000000000..71d88c4a6a8743 --- /dev/null +++ b/packages/warning/README.md @@ -0,0 +1,44 @@ +# Warning + +Utility for warning messages to the console based on a passed condition. + +## Installation + +Install the module + +```bash +npm install @wordpress/warning --save +``` + +This is recommended to use in conjunction with [`babel-plugin-dev-expression`](https://github.com/4Catalyzer/babel-plugin-dev-expression) so your warning messages won't end up in your production bundle. + +_This package assumes that your code will run in an **ES2015+** environment. If you're using an environment that has limited or no support for ES2015+ such as lower versions of IE then using [core-js](https://github.com/zloirock/core-js) or [@babel/polyfill](https://babeljs.io/docs/en/next/babel-polyfill) will add support for these methods. Learn more about it in [Babel docs](https://babeljs.io/docs/en/next/caveats)._ + +## API + + + +# **default** + +Shows a warning with `messages` if `condition` passes and environment is not `production`. + +_Usage_ + +```js +import warning from '@wordpress/warning'; + +function MyComponent( props ) { + warning( ! props.title, '`props.title` was not passed' ); + ... +} +``` + +_Parameters_ + +- _condition_ `boolean`: Whether the warning will be triggered or not. +- _messages_ `Array`: Message(s) to show in the warning. + + + + +

Code is Poetry.

diff --git a/packages/warning/package.json b/packages/warning/package.json new file mode 100644 index 00000000000000..076f28d16150ed --- /dev/null +++ b/packages/warning/package.json @@ -0,0 +1,27 @@ +{ + "name": "@wordpress/warning", + "version": "0.0.1", + "description": "Warning utility for WordPress.", + "author": "The WordPress Contributors", + "license": "GPL-2.0-or-later", + "keywords": [ + "wordpress", + "warning" + ], + "homepage": "https://github.com/WordPress/gutenberg/tree/master/packages/warning/README.md", + "repository": { + "type": "git", + "url": "https://github.com/WordPress/gutenberg.git", + "directory": "packages/warning" + }, + "bugs": { + "url": "https://github.com/WordPress/gutenberg/issues" + }, + "main": "build/index.js", + "module": "build-module/index.js", + "react-native": "src/index", + "sideEffects": false, + "publishConfig": { + "access": "public" + } +} diff --git a/packages/warning/src/index.js b/packages/warning/src/index.js new file mode 100644 index 00000000000000..8e20b9fd238a4f --- /dev/null +++ b/packages/warning/src/index.js @@ -0,0 +1,37 @@ +/** + * Shows a warning with `messages` if `condition` passes and environment is not `production`. + * + * @param {boolean} condition Whether the warning will be triggered or not. + * @param {string[]} messages Message(s) to show in the warning. + * + * @example + * ```js + * import warning from '@wordpress/warning'; + * + * function MyComponent( props ) { + * warning( ! props.title, '`props.title` was not passed' ); + * ... + * } + * ``` + */ +export default function warning( condition, ...messages ) { + if ( process.env.NODE_ENV !== 'production' ) { + if ( ! condition ) { + return; + } + + const text = messages.join( '\n' ); + + // eslint-disable-next-line no-console + console.warn( text ); + + // Throwing an error and catching it immediately to improve debugging + // A consumer can use 'pause on caught exceptions' + // https://github.com/facebook/react/issues/4216 + try { + throw Error( text ); + } catch ( x ) { + // do nothing + } + } +} diff --git a/packages/warning/src/test/index.js b/packages/warning/src/test/index.js new file mode 100644 index 00000000000000..a2318eb56ab591 --- /dev/null +++ b/packages/warning/src/test/index.js @@ -0,0 +1,30 @@ +/** + * Internal dependencies + */ +import warning from '..'; + +const initialNodeEnv = process.env.NODE_ENV; + +describe( 'warning', () => { + afterEach( () => { + process.env.NODE_ENV = initialNodeEnv; + } ); + + it( 'logs to console.warn when NODE_ENV is not "production"', () => { + process.env.NODE_ENV = 'development'; + warning( true, 'warn', 'ing' ); + expect( console ).toHaveWarnedWith( 'warn\ning' ); + } ); + + it( 'does not log to console.warn if NODE_ENV is "production"', () => { + process.env.NODE_ENV = 'production'; + warning( true, 'warn', 'ing' ); + expect( console ).not.toHaveWarned(); + } ); + + it( 'does not log to console.warn if condition is falsy', () => { + process.env.NODE_ENV = 'development'; + warning( false, 'warn', 'ing' ); + expect( console ).not.toHaveWarned(); + } ); +} ); From 6649664f5595a5196214e604066f6e65fe0e7191 Mon Sep 17 00:00:00 2001 From: Haz Date: Tue, 24 Dec 2019 00:16:03 -0300 Subject: [PATCH 02/18] Add babel-plugin-dev-expression --- babel.config.js | 1 + package-lock.json | 13 +++++++++++-- package.json | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/babel.config.js b/babel.config.js index 83d78a0eeadb6d..2e97daeab5c807 100644 --- a/babel.config.js +++ b/babel.config.js @@ -6,6 +6,7 @@ module.exports = function( api ) { plugins: [ 'babel-plugin-emotion', 'babel-plugin-inline-json-import', + 'babel-plugin-dev-expression', ], }; }; diff --git a/package-lock.json b/package-lock.json index 0c7e5790f2bb60..b0f4469dcbdd61 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9001,6 +9001,9 @@ "lodash": "^4.17.15" } }, + "@wordpress/warning": { + "version": "file:packages/warning" + }, "@wordpress/wordcount": { "version": "file:packages/wordcount", "requires": { @@ -10383,6 +10386,12 @@ "@mdx-js/util": "^1.5.1" } }, + "babel-plugin-dev-expression": { + "version": "0.2.2", + "resolved": "https://registry.npmjs.org/babel-plugin-dev-expression/-/babel-plugin-dev-expression-0.2.2.tgz", + "integrity": "sha512-y32lfBif+c2FIh5dwGfcc/IfX5aw/Bru7Du7W2n17sJE/GJGAsmIk5DPW/8JOoeKpXW5evJfJOvRq5xkiS6vng==", + "dev": true + }, "babel-plugin-dynamic-import-node": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/babel-plugin-dynamic-import-node/-/babel-plugin-dynamic-import-node-2.3.0.tgz", @@ -22852,7 +22861,7 @@ "dependencies": { "clone-deep": { "version": "0.2.4", - "resolved": "http://registry.npmjs.org/clone-deep/-/clone-deep-0.2.4.tgz", + "resolved": "https://registry.npmjs.org/clone-deep/-/clone-deep-0.2.4.tgz", "integrity": "sha1-TnPdCen7lxzDhnDF3O2cGJZIHMY=", "dev": true, "requires": { @@ -22886,7 +22895,7 @@ "dependencies": { "kind-of": { "version": "2.0.1", - "resolved": "http://registry.npmjs.org/kind-of/-/kind-of-2.0.1.tgz", + "resolved": "https://registry.npmjs.org/kind-of/-/kind-of-2.0.1.tgz", "integrity": "sha1-AY7HpM5+OobLkUG+UZ0kyPqpgbU=", "dev": true, "requires": { diff --git a/package.json b/package.json index 2b5fce99c856b1..2aaf6aef71276f 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "@wordpress/token-list": "file:packages/token-list", "@wordpress/url": "file:packages/url", "@wordpress/viewport": "file:packages/viewport", + "@wordpress/warning": "file:packages/warning", "@wordpress/wordcount": "file:packages/wordcount" }, "devDependencies": { @@ -102,6 +103,7 @@ "@wordpress/postcss-themes": "file:packages/postcss-themes", "@wordpress/scripts": "file:packages/scripts", "babel-loader": "8.0.6", + "babel-plugin-dev-expression": "0.2.2", "babel-plugin-emotion": "10.0.23", "babel-plugin-inline-json-import": "0.3.2", "babel-plugin-react-native-classname-to-style": "1.2.2", From e3ba9d7f3e72761e612c152ce58883a7da5c35f1 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 26 Dec 2019 00:10:50 -0300 Subject: [PATCH 03/18] Remove babel-plugin-dev-expression --- babel.config.js | 1 - package-lock.json | 6 ------ package.json | 1 - 3 files changed, 8 deletions(-) diff --git a/babel.config.js b/babel.config.js index 2e97daeab5c807..83d78a0eeadb6d 100644 --- a/babel.config.js +++ b/babel.config.js @@ -6,7 +6,6 @@ module.exports = function( api ) { plugins: [ 'babel-plugin-emotion', 'babel-plugin-inline-json-import', - 'babel-plugin-dev-expression', ], }; }; diff --git a/package-lock.json b/package-lock.json index b0f4469dcbdd61..1fafa129daea2e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10386,12 +10386,6 @@ "@mdx-js/util": "^1.5.1" } }, - "babel-plugin-dev-expression": { - "version": "0.2.2", - "resolved": "https://registry.npmjs.org/babel-plugin-dev-expression/-/babel-plugin-dev-expression-0.2.2.tgz", - "integrity": "sha512-y32lfBif+c2FIh5dwGfcc/IfX5aw/Bru7Du7W2n17sJE/GJGAsmIk5DPW/8JOoeKpXW5evJfJOvRq5xkiS6vng==", - "dev": true - }, "babel-plugin-dynamic-import-node": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/babel-plugin-dynamic-import-node/-/babel-plugin-dynamic-import-node-2.3.0.tgz", diff --git a/package.json b/package.json index 2aaf6aef71276f..69f6d53f4587b5 100644 --- a/package.json +++ b/package.json @@ -103,7 +103,6 @@ "@wordpress/postcss-themes": "file:packages/postcss-themes", "@wordpress/scripts": "file:packages/scripts", "babel-loader": "8.0.6", - "babel-plugin-dev-expression": "0.2.2", "babel-plugin-emotion": "10.0.23", "babel-plugin-inline-json-import": "0.3.2", "babel-plugin-react-native-classname-to-style": "1.2.2", From a2732ecc9fd616b9ac4e2513363a0b1dca48b662 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 26 Dec 2019 03:01:05 -0300 Subject: [PATCH 04/18] Add babel-plugin to the warning pacakge --- packages/warning/README.md | 6 +- packages/warning/babel-plugin.js | 1 + packages/warning/src/babel-plugin.js | 62 +++++++++++++ packages/warning/src/test/babel-plugin.js | 102 ++++++++++++++++++++++ 4 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 packages/warning/babel-plugin.js create mode 100644 packages/warning/src/babel-plugin.js create mode 100644 packages/warning/src/test/babel-plugin.js diff --git a/packages/warning/README.md b/packages/warning/README.md index 71d88c4a6a8743..053465f033692f 100644 --- a/packages/warning/README.md +++ b/packages/warning/README.md @@ -10,10 +10,12 @@ Install the module npm install @wordpress/warning --save ``` -This is recommended to use in conjunction with [`babel-plugin-dev-expression`](https://github.com/4Catalyzer/babel-plugin-dev-expression) so your warning messages won't end up in your production bundle. - _This package assumes that your code will run in an **ES2015+** environment. If you're using an environment that has limited or no support for ES2015+ such as lower versions of IE then using [core-js](https://github.com/zloirock/core-js) or [@babel/polyfill](https://babeljs.io/docs/en/next/babel-polyfill) will add support for these methods. Learn more about it in [Babel docs](https://babeljs.io/docs/en/next/caveats)._ +## Reducing bundle size + +Literal strings aren't minified. Keeping them in your production bundle may increase the bundle size significantly. To prevent that, you can put `@wordpress/warning/babel-plugin` into your [babel config](https://babeljs.io/docs/en/plugins#plugin-options) or use [`@wordpress/babel-preset-default`](https://www.npmjs.com/package/@wordpress/babel-preset-default), which already includes the babel plugin. + ## API diff --git a/packages/warning/babel-plugin.js b/packages/warning/babel-plugin.js new file mode 100644 index 00000000000000..65b687a8b3e053 --- /dev/null +++ b/packages/warning/babel-plugin.js @@ -0,0 +1 @@ +module.exports = require( './src/babel-plugin' ); diff --git a/packages/warning/src/babel-plugin.js b/packages/warning/src/babel-plugin.js new file mode 100644 index 00000000000000..12eb65f699585a --- /dev/null +++ b/packages/warning/src/babel-plugin.js @@ -0,0 +1,62 @@ +const pkg = require( '../package.json' ); + +function babelPlugin( { types: t } ) { + const seen = Symbol(); + + const binaryExpression = t.binaryExpression( + '!==', + t.memberExpression( + t.memberExpression( t.identifier( 'process' ), t.identifier( 'env' ), false ), + t.identifier( 'NODE_ENV' ), + false + ), + t.stringLiteral( 'production' ) + ); + + return { + visitor: { + ImportDeclaration( path, state ) { + const { node } = path; + const isThisPackageImport = node.source.value.indexOf( pkg.name ) !== -1; + + if ( ! isThisPackageImport ) { + return; + } + + const defaultSpecifier = node.specifiers.find( + ( specifier ) => specifier.type === 'ImportDefaultSpecifier' + ); + + const { name } = defaultSpecifier.local; + + state.callee = name; + }, + CallExpression( path, state ) { + const { node } = path; + + // Ignore if it's already been processed + if ( node[ seen ] ) { + return; + } + + const name = state.callee || state.opts.callee; + + if ( path.get( 'callee' ).isIdentifier( { name } ) ) { + // Turns this code: + // warning(condition, argument, argument); + // into this: + // process.env.NODE_ENV !== "production" ? warning(condition, argument, argument) : void 0; + node[ seen ] = true; + path.replaceWith( + t.ifStatement( + binaryExpression, + t.blockStatement( [ t.expressionStatement( node ) ] ) + ) + ); + } + }, + }, + }; +} + +module.exports = babelPlugin; diff --git a/packages/warning/src/test/babel-plugin.js b/packages/warning/src/test/babel-plugin.js new file mode 100644 index 00000000000000..1ed9b2f556aa51 --- /dev/null +++ b/packages/warning/src/test/babel-plugin.js @@ -0,0 +1,102 @@ +/** + * External dependencies + */ +import { transform } from '@babel/core'; + +/** + * Internal dependencies + */ +import babelPlugin from '../babel-plugin'; + +function join( ...strings ) { + return strings.join( '\n' ); +} + +function compare( input, output, options = {} ) { + const { code } = transform( input, { + configFile: false, + plugins: [ [ babelPlugin, options ] ], + } ); + expect( code ).toEqual( output ); +} + +describe( 'babel-plugin', function() { + it( 'should replace warning calls with import declaration', () => { + compare( + join( + 'import warning from "@wordpress/warning";', + 'warning(true, "a", "b");' + ), + join( + 'import warning from "@wordpress/warning";', + 'process.env.NODE_ENV !== "production" ? warning(true, "a", "b") : void 0;' + ) + ); + } ); + + it( 'should not replace warning calls without import declaration', () => { + compare( + 'warning(true, "a", "b");', + 'warning(true, "a", "b");' + ); + } ); + + it( 'should replace warning calls without import declaration with plugin options', () => { + compare( + 'warning(true, "a", "b");', + 'process.env.NODE_ENV !== "production" ? warning(true, "a", "b") : void 0;', + { callee: 'warning' } + ); + } ); + + it( 'should replace multiple warning calls', () => { + compare( + join( + 'import warning from "@wordpress/warning";', + 'warning(true, "a", "b");', + 'warning(false, "b", "a");', + 'warning(cond, "c");', + ), + join( + 'import warning from "@wordpress/warning";', + 'process.env.NODE_ENV !== "production" ? warning(true, "a", "b") : void 0;', + 'process.env.NODE_ENV !== "production" ? warning(false, "b", "a") : void 0;', + 'process.env.NODE_ENV !== "production" ? warning(cond, "c") : void 0;' + ) + ); + } ); + + it( 'should identify warning callee name', () => { + compare( + join( + 'import warn from "@wordpress/warning";', + 'warn(true, "a", "b");', + 'warn(false, "b", "a");', + 'warn(cond, "c");', + ), + join( + 'import warn from "@wordpress/warning";', + 'process.env.NODE_ENV !== "production" ? warn(true, "a", "b") : void 0;', + 'process.env.NODE_ENV !== "production" ? warn(false, "b", "a") : void 0;', + 'process.env.NODE_ENV !== "production" ? warn(cond, "c") : void 0;' + ) + ); + } ); + + it( 'should identify warning callee name by ', () => { + compare( + join( + 'import warn from "@wordpress/warning";', + 'warn(true, "a", "b");', + 'warn(false, "b", "a");', + 'warn(cond, "c");', + ), + join( + 'import warn from "@wordpress/warning";', + 'process.env.NODE_ENV !== "production" ? warn(true, "a", "b") : void 0;', + 'process.env.NODE_ENV !== "production" ? warn(false, "b", "a") : void 0;', + 'process.env.NODE_ENV !== "production" ? warn(cond, "c") : void 0;' + ) + ); + } ); +} ); From 93eddbba3a45bd87b57cf9432db9a324dad55969 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 26 Dec 2019 03:01:52 -0300 Subject: [PATCH 05/18] Include @wordpress/warning/babel-plugin into @wordpress/babel-preset-default --- package-lock.json | 1 + packages/babel-preset-default/index.js | 1 + packages/babel-preset-default/package.json | 1 + 3 files changed, 3 insertions(+) diff --git a/package-lock.json b/package-lock.json index 1fafa129daea2e..d015c3101ae5dc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7556,6 +7556,7 @@ "@wordpress/babel-plugin-import-jsx-pragma": "file:packages/babel-plugin-import-jsx-pragma", "@wordpress/browserslist-config": "file:packages/browserslist-config", "@wordpress/element": "file:packages/element", + "@wordpress/warning": "file:packages/warning", "core-js": "^3.1.4" } }, diff --git a/packages/babel-preset-default/index.js b/packages/babel-preset-default/index.js index a1343706b03b8b..6d368c70f0d00b 100644 --- a/packages/babel-preset-default/index.js +++ b/packages/babel-preset-default/index.js @@ -56,6 +56,7 @@ module.exports = function( api ) { presets: [ getPresetEnv() ], plugins: [ require.resolve( '@babel/plugin-proposal-object-rest-spread' ), + require.resolve( '@wordpress/warning/babel-plugin' ), [ require.resolve( '@wordpress/babel-plugin-import-jsx-pragma' ), { diff --git a/packages/babel-preset-default/package.json b/packages/babel-preset-default/package.json index 9c6cf0263496da..fc38a874e780de 100644 --- a/packages/babel-preset-default/package.json +++ b/packages/babel-preset-default/package.json @@ -37,6 +37,7 @@ "@wordpress/babel-plugin-import-jsx-pragma": "file:../babel-plugin-import-jsx-pragma", "@wordpress/browserslist-config": "file:../browserslist-config", "@wordpress/element": "file:../element", + "@wordpress/warning": "file:../warning", "core-js": "^3.1.4" }, "publishConfig": { From e357a175e4f97c052b958441cc6c7e1b360d6a34 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 26 Dec 2019 03:08:27 -0300 Subject: [PATCH 06/18] Add warning to webpack.config.js --- webpack.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/webpack.config.js b/webpack.config.js index f1b9c578192eba..586a28df6d5001 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -90,6 +90,7 @@ module.exports = { 'token-list', 'server-side-render', 'shortcode', + 'warning', ].map( camelCaseDash ) ), new CopyWebpackPlugin( gutenbergPackages.map( ( packageName ) => ( { From be1f9213ddb3cf4a2fbca0ca20e08e81edad3ce4 Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 8 Jan 2020 22:01:24 -0300 Subject: [PATCH 07/18] Check the presence of process.env --- packages/warning/src/index.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/warning/src/index.js b/packages/warning/src/index.js index 8e20b9fd238a4f..66d8b6c69411d9 100644 --- a/packages/warning/src/index.js +++ b/packages/warning/src/index.js @@ -1,3 +1,11 @@ +function isDev() { + return ( + typeof process !== 'undefined' && + process.env && + process.env.NODE_ENV !== 'production' + ); +} + /** * Shows a warning with `messages` if `condition` passes and environment is not `production`. * @@ -15,7 +23,7 @@ * ``` */ export default function warning( condition, ...messages ) { - if ( process.env.NODE_ENV !== 'production' ) { + if ( isDev() ) { if ( ! condition ) { return; } From 0765740a0701f1bdab1be9a307a360ca0937bb55 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 9 Jan 2020 18:38:39 -0300 Subject: [PATCH 08/18] Check presence of process.env in the code transformed by the babel plugin --- packages/warning/src/babel-plugin.js | 30 +++++++++++++++++------ packages/warning/src/test/babel-plugin.js | 22 ++++++++--------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/packages/warning/src/babel-plugin.js b/packages/warning/src/babel-plugin.js index 12eb65f699585a..85577f32f9001a 100644 --- a/packages/warning/src/babel-plugin.js +++ b/packages/warning/src/babel-plugin.js @@ -3,16 +3,30 @@ const pkg = require( '../package.json' ); function babelPlugin( { types: t } ) { const seen = Symbol(); - const binaryExpression = t.binaryExpression( + const typeofProcessExpression = t.binaryExpression( '!==', - t.memberExpression( - t.memberExpression( t.identifier( 'process' ), t.identifier( 'env' ), false ), - t.identifier( 'NODE_ENV' ), - false - ), + t.unaryExpression( 'typeof', t.identifier( 'process' ), false ), + t.stringLiteral( 'undefined' ) + ); + + const processEnvExpression = t.memberExpression( + t.identifier( 'process' ), + t.identifier( 'env' ), + false + ); + + const nodeEnvCheckExpression = t.binaryExpression( + '!==', + t.memberExpression( processEnvExpression, t.identifier( 'NODE_ENV' ), false ), t.stringLiteral( 'production' ) ); + const logicalExpression = t.logicalExpression( + '&&', + t.logicalExpression( '&&', typeofProcessExpression, processEnvExpression ), + nodeEnvCheckExpression + ); + return { visitor: { ImportDeclaration( path, state ) { @@ -45,11 +59,11 @@ function babelPlugin( { types: t } ) { // Turns this code: // warning(condition, argument, argument); // into this: - // process.env.NODE_ENV !== "production" ? warning(condition, argument, argument) : void 0; + // typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warning(condition, argument, argument) : void 0; node[ seen ] = true; path.replaceWith( t.ifStatement( - binaryExpression, + logicalExpression, t.blockStatement( [ t.expressionStatement( node ) ] ) ) ); diff --git a/packages/warning/src/test/babel-plugin.js b/packages/warning/src/test/babel-plugin.js index 1ed9b2f556aa51..7758af5a96b9c4 100644 --- a/packages/warning/src/test/babel-plugin.js +++ b/packages/warning/src/test/babel-plugin.js @@ -29,7 +29,7 @@ describe( 'babel-plugin', function() { ), join( 'import warning from "@wordpress/warning";', - 'process.env.NODE_ENV !== "production" ? warning(true, "a", "b") : void 0;' + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warning(true, "a", "b") : void 0;' ) ); } ); @@ -44,7 +44,7 @@ describe( 'babel-plugin', function() { it( 'should replace warning calls without import declaration with plugin options', () => { compare( 'warning(true, "a", "b");', - 'process.env.NODE_ENV !== "production" ? warning(true, "a", "b") : void 0;', + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warning(true, "a", "b") : void 0;', { callee: 'warning' } ); } ); @@ -59,9 +59,9 @@ describe( 'babel-plugin', function() { ), join( 'import warning from "@wordpress/warning";', - 'process.env.NODE_ENV !== "production" ? warning(true, "a", "b") : void 0;', - 'process.env.NODE_ENV !== "production" ? warning(false, "b", "a") : void 0;', - 'process.env.NODE_ENV !== "production" ? warning(cond, "c") : void 0;' + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warning(true, "a", "b") : void 0;', + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warning(false, "b", "a") : void 0;', + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warning(cond, "c") : void 0;' ) ); } ); @@ -76,9 +76,9 @@ describe( 'babel-plugin', function() { ), join( 'import warn from "@wordpress/warning";', - 'process.env.NODE_ENV !== "production" ? warn(true, "a", "b") : void 0;', - 'process.env.NODE_ENV !== "production" ? warn(false, "b", "a") : void 0;', - 'process.env.NODE_ENV !== "production" ? warn(cond, "c") : void 0;' + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warn(true, "a", "b") : void 0;', + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warn(false, "b", "a") : void 0;', + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warn(cond, "c") : void 0;' ) ); } ); @@ -93,9 +93,9 @@ describe( 'babel-plugin', function() { ), join( 'import warn from "@wordpress/warning";', - 'process.env.NODE_ENV !== "production" ? warn(true, "a", "b") : void 0;', - 'process.env.NODE_ENV !== "production" ? warn(false, "b", "a") : void 0;', - 'process.env.NODE_ENV !== "production" ? warn(cond, "c") : void 0;' + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warn(true, "a", "b") : void 0;', + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warn(false, "b", "a") : void 0;', + 'typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warn(cond, "c") : void 0;' ) ); } ); From aa2d2005f805801a150fd95bd18a775a14fd434f Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 9 Jan 2020 18:43:20 -0300 Subject: [PATCH 09/18] Move babel-plugin.js and its test file to the directory top-level --- packages/warning/babel-plugin.js | 77 ++++++++++++++++++- packages/warning/src/babel-plugin.js | 76 ------------------ .../warning/{src => }/test/babel-plugin.js | 0 3 files changed, 76 insertions(+), 77 deletions(-) delete mode 100644 packages/warning/src/babel-plugin.js rename packages/warning/{src => }/test/babel-plugin.js (100%) diff --git a/packages/warning/babel-plugin.js b/packages/warning/babel-plugin.js index 65b687a8b3e053..8908ca9e66d665 100644 --- a/packages/warning/babel-plugin.js +++ b/packages/warning/babel-plugin.js @@ -1 +1,76 @@ -module.exports = require( './src/babel-plugin' ); +const pkg = require( './package.json' ); + +function babelPlugin( { types: t } ) { + const seen = Symbol(); + + const typeofProcessExpression = t.binaryExpression( + '!==', + t.unaryExpression( 'typeof', t.identifier( 'process' ), false ), + t.stringLiteral( 'undefined' ) + ); + + const processEnvExpression = t.memberExpression( + t.identifier( 'process' ), + t.identifier( 'env' ), + false + ); + + const nodeEnvCheckExpression = t.binaryExpression( + '!==', + t.memberExpression( processEnvExpression, t.identifier( 'NODE_ENV' ), false ), + t.stringLiteral( 'production' ) + ); + + const logicalExpression = t.logicalExpression( + '&&', + t.logicalExpression( '&&', typeofProcessExpression, processEnvExpression ), + nodeEnvCheckExpression + ); + + return { + visitor: { + ImportDeclaration( path, state ) { + const { node } = path; + const isThisPackageImport = node.source.value.indexOf( pkg.name ) !== -1; + + if ( ! isThisPackageImport ) { + return; + } + + const defaultSpecifier = node.specifiers.find( + ( specifier ) => specifier.type === 'ImportDefaultSpecifier' + ); + + const { name } = defaultSpecifier.local; + + state.callee = name; + }, + CallExpression( path, state ) { + const { node } = path; + + // Ignore if it's already been processed + if ( node[ seen ] ) { + return; + } + + const name = state.callee || state.opts.callee; + + if ( path.get( 'callee' ).isIdentifier( { name } ) ) { + // Turns this code: + // warning(condition, argument, argument); + // into this: + // typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warning(condition, argument, argument) : void 0; + node[ seen ] = true; + path.replaceWith( + t.ifStatement( + logicalExpression, + t.blockStatement( [ t.expressionStatement( node ) ] ) + ) + ); + } + }, + }, + }; +} + +module.exports = babelPlugin; diff --git a/packages/warning/src/babel-plugin.js b/packages/warning/src/babel-plugin.js deleted file mode 100644 index 85577f32f9001a..00000000000000 --- a/packages/warning/src/babel-plugin.js +++ /dev/null @@ -1,76 +0,0 @@ -const pkg = require( '../package.json' ); - -function babelPlugin( { types: t } ) { - const seen = Symbol(); - - const typeofProcessExpression = t.binaryExpression( - '!==', - t.unaryExpression( 'typeof', t.identifier( 'process' ), false ), - t.stringLiteral( 'undefined' ) - ); - - const processEnvExpression = t.memberExpression( - t.identifier( 'process' ), - t.identifier( 'env' ), - false - ); - - const nodeEnvCheckExpression = t.binaryExpression( - '!==', - t.memberExpression( processEnvExpression, t.identifier( 'NODE_ENV' ), false ), - t.stringLiteral( 'production' ) - ); - - const logicalExpression = t.logicalExpression( - '&&', - t.logicalExpression( '&&', typeofProcessExpression, processEnvExpression ), - nodeEnvCheckExpression - ); - - return { - visitor: { - ImportDeclaration( path, state ) { - const { node } = path; - const isThisPackageImport = node.source.value.indexOf( pkg.name ) !== -1; - - if ( ! isThisPackageImport ) { - return; - } - - const defaultSpecifier = node.specifiers.find( - ( specifier ) => specifier.type === 'ImportDefaultSpecifier' - ); - - const { name } = defaultSpecifier.local; - - state.callee = name; - }, - CallExpression( path, state ) { - const { node } = path; - - // Ignore if it's already been processed - if ( node[ seen ] ) { - return; - } - - const name = state.callee || state.opts.callee; - - if ( path.get( 'callee' ).isIdentifier( { name } ) ) { - // Turns this code: - // warning(condition, argument, argument); - // into this: - // typeof process !== "undefined" && process.env && process.env.NODE_ENV !== "production" ? warning(condition, argument, argument) : void 0; - node[ seen ] = true; - path.replaceWith( - t.ifStatement( - logicalExpression, - t.blockStatement( [ t.expressionStatement( node ) ] ) - ) - ); - } - }, - }, - }; -} - -module.exports = babelPlugin; diff --git a/packages/warning/src/test/babel-plugin.js b/packages/warning/test/babel-plugin.js similarity index 100% rename from packages/warning/src/test/babel-plugin.js rename to packages/warning/test/babel-plugin.js From b045842ba269f0539f3327e947e40f521fa86207 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 9 Jan 2020 18:59:37 -0300 Subject: [PATCH 10/18] Improve README with information about dead code removal --- packages/warning/README.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/warning/README.md b/packages/warning/README.md index 053465f033692f..684dbaf37f41bf 100644 --- a/packages/warning/README.md +++ b/packages/warning/README.md @@ -14,7 +14,17 @@ _This package assumes that your code will run in an **ES2015+** environment. If ## Reducing bundle size -Literal strings aren't minified. Keeping them in your production bundle may increase the bundle size significantly. To prevent that, you can put `@wordpress/warning/babel-plugin` into your [babel config](https://babeljs.io/docs/en/plugins#plugin-options) or use [`@wordpress/babel-preset-default`](https://www.npmjs.com/package/@wordpress/babel-preset-default), which already includes the babel plugin. +Literal strings aren't minified. Keeping them in your production bundle may increase the bundle size significantly. + +To prevent that, you should: + +1. Put `@wordpress/warning/babel-plugin` into your [babel config](https://babeljs.io/docs/en/plugins#plugin-options) or use [`@wordpress/babel-preset-default`](https://www.npmjs.com/package/@wordpress/babel-preset-default), which already includes the babel plugin. + + This will make sure your `warning` calls are wrapped within a condition that checks if `process.env.NODE_ENV !== 'production'`. + +2. Use [UglifyJS](https://github.com/mishoo/UglifyJS2), [Terser](https://github.com/terser/terser) or any other JavaScript parser that performs [dead code elimination](https://en.wikipedia.org/wiki/Dead_code_elimination). This is usually used in conjunction with JavaScript bundlers, such as [webpack](https://github.com/webpack/webpack). + + When parsing the code in `production` mode, the `warning` call will be removed altogether. ## API From 47cce9f3f28ca6a1121d32d0f628d3d0d5c78324 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 9 Jan 2020 19:06:07 -0300 Subject: [PATCH 11/18] Turn sideEffects into true in package.json --- packages/warning/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/warning/package.json b/packages/warning/package.json index 076f28d16150ed..957148d3eeab42 100644 --- a/packages/warning/package.json +++ b/packages/warning/package.json @@ -20,7 +20,7 @@ "main": "build/index.js", "module": "build-module/index.js", "react-native": "src/index", - "sideEffects": false, + "sideEffects": true, "publishConfig": { "access": "public" } From bb8209ac2cd45b5ff9d0bc0a2baaea510274f5b3 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 9 Jan 2020 19:10:02 -0300 Subject: [PATCH 12/18] Revert sideEffects change --- packages/warning/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/warning/package.json b/packages/warning/package.json index 957148d3eeab42..076f28d16150ed 100644 --- a/packages/warning/package.json +++ b/packages/warning/package.json @@ -20,7 +20,7 @@ "main": "build/index.js", "module": "build-module/index.js", "react-native": "src/index", - "sideEffects": true, + "sideEffects": false, "publishConfig": { "access": "public" } From d046ef653cc8f4f677fd6fb72ad0f4ccb805297b Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 9 Jan 2020 19:46:02 -0300 Subject: [PATCH 13/18] npm run docs:build --- packages/warning/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/warning/README.md b/packages/warning/README.md index 684dbaf37f41bf..ae61c55eff9a46 100644 --- a/packages/warning/README.md +++ b/packages/warning/README.md @@ -18,11 +18,11 @@ Literal strings aren't minified. Keeping them in your production bundle may incr To prevent that, you should: -1. Put `@wordpress/warning/babel-plugin` into your [babel config](https://babeljs.io/docs/en/plugins#plugin-options) or use [`@wordpress/babel-preset-default`](https://www.npmjs.com/package/@wordpress/babel-preset-default), which already includes the babel plugin. +1. Put `@wordpress/warning/babel-plugin` into your [babel config](https://babeljs.io/docs/en/plugins#plugin-options) or use [`@wordpress/babel-preset-default`](https://www.npmjs.com/package/@wordpress/babel-preset-default), which already includes the babel plugin. This will make sure your `warning` calls are wrapped within a condition that checks if `process.env.NODE_ENV !== 'production'`. -2. Use [UglifyJS](https://github.com/mishoo/UglifyJS2), [Terser](https://github.com/terser/terser) or any other JavaScript parser that performs [dead code elimination](https://en.wikipedia.org/wiki/Dead_code_elimination). This is usually used in conjunction with JavaScript bundlers, such as [webpack](https://github.com/webpack/webpack). +2. Use [UglifyJS](https://github.com/mishoo/UglifyJS2), [Terser](https://github.com/terser/terser) or any other JavaScript parser that performs [dead code elimination](https://en.wikipedia.org/wiki/Dead_code_elimination). This is usually used in conjunction with JavaScript bundlers, such as [webpack](https://github.com/webpack/webpack). When parsing the code in `production` mode, the `warning` call will be removed altogether. From 25dfd7f06ca4a3346ab6e1ee8f29797be6e5055c Mon Sep 17 00:00:00 2001 From: Haz Date: Mon, 13 Jan 2020 16:23:13 -0300 Subject: [PATCH 14/18] Update packages/warning/babel-plugin.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Grzegorz (Greg) Ziółkowski --- packages/warning/babel-plugin.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/warning/babel-plugin.js b/packages/warning/babel-plugin.js index 8908ca9e66d665..6e83e909b41852 100644 --- a/packages/warning/babel-plugin.js +++ b/packages/warning/babel-plugin.js @@ -1,3 +1,6 @@ +/** + * Internal dependencies + */ const pkg = require( './package.json' ); function babelPlugin( { types: t } ) { From 44cdafec2adc595d45c7f1e199d0ca54546b63aa Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 14 Jan 2020 11:24:21 -0500 Subject: [PATCH 15/18] warning: Add types checking for warning package --- packages/warning/babel-plugin.js | 8 ++++++++ tsconfig.json | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/warning/babel-plugin.js b/packages/warning/babel-plugin.js index 6e83e909b41852..4eca0896d74655 100644 --- a/packages/warning/babel-plugin.js +++ b/packages/warning/babel-plugin.js @@ -3,6 +3,14 @@ */ const pkg = require( './package.json' ); +/** + * Babel plugin which transforms `warning` function calls to wrap within a + * condition that checks if `process.env.NODE_ENV !== 'production'`. + * + * @param {import('@babel/core')} babel Current Babel object. + * + * @return {import('@babel/core').PluginObj} Babel plugin object. + */ function babelPlugin( { types: t } ) { const seen = Symbol(); diff --git a/tsconfig.json b/tsconfig.json index a1854349a9d95d..6e74bf45066430 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -38,7 +38,8 @@ "./packages/is-shallow-equal/**/*.js", "./packages/priority-queue/**/*.js", "./packages/token-list/**/*.js", - "./packages/url/**/*.js" + "./packages/url/**/*.js", + "./packages/warning/**/*.js", ], "exclude": [ "./packages/*/benchmark", From f0bf76569bb7cd93bd7677bb75259aacbdd71bf9 Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 15 Jan 2020 00:42:16 -0300 Subject: [PATCH 16/18] Avoid indentation on the warning function with an early return --- packages/warning/src/index.js | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/warning/src/index.js b/packages/warning/src/index.js index 66d8b6c69411d9..34ac519f5f741c 100644 --- a/packages/warning/src/index.js +++ b/packages/warning/src/index.js @@ -23,23 +23,25 @@ function isDev() { * ``` */ export default function warning( condition, ...messages ) { - if ( isDev() ) { - if ( ! condition ) { - return; - } + if ( ! isDev() ) { + return; + } + + if ( ! condition ) { + return; + } - const text = messages.join( '\n' ); + const text = messages.join( '\n' ); - // eslint-disable-next-line no-console - console.warn( text ); + // eslint-disable-next-line no-console + console.warn( text ); - // Throwing an error and catching it immediately to improve debugging - // A consumer can use 'pause on caught exceptions' - // https://github.com/facebook/react/issues/4216 - try { - throw Error( text ); - } catch ( x ) { - // do nothing - } + // Throwing an error and catching it immediately to improve debugging + // A consumer can use 'pause on caught exceptions' + // https://github.com/facebook/react/issues/4216 + try { + throw Error( text ); + } catch ( x ) { + // do nothing } } From b389b348102615ed849f8b20f609347c4a0d410e Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 15 Jan 2020 00:48:22 -0300 Subject: [PATCH 17/18] Fix TS error on babel-plugin.js --- packages/warning/babel-plugin.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/warning/babel-plugin.js b/packages/warning/babel-plugin.js index 4eca0896d74655..2266fe0a0d416e 100644 --- a/packages/warning/babel-plugin.js +++ b/packages/warning/babel-plugin.js @@ -52,9 +52,10 @@ function babelPlugin( { types: t } ) { ( specifier ) => specifier.type === 'ImportDefaultSpecifier' ); - const { name } = defaultSpecifier.local; - - state.callee = name; + if ( defaultSpecifier && defaultSpecifier.local ) { + const { name } = defaultSpecifier.local; + state.callee = name; + } }, CallExpression( path, state ) { const { node } = path; From 17243467b70b2c2f0c7e404d1c510bc3faf13d6c Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 15 Jan 2020 01:01:18 -0300 Subject: [PATCH 18/18] Accept a single string as message instead of multiple messages --- packages/warning/README.md | 4 ++-- packages/warning/src/index.js | 12 +++++------- packages/warning/src/test/index.js | 8 ++++---- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/warning/README.md b/packages/warning/README.md index ae61c55eff9a46..74d5d2d7f5cd34 100644 --- a/packages/warning/README.md +++ b/packages/warning/README.md @@ -32,7 +32,7 @@ To prevent that, you should: # **default** -Shows a warning with `messages` if `condition` passes and environment is not `production`. +Shows a warning with `message` if `condition` passes and environment is not `production`. _Usage_ @@ -48,7 +48,7 @@ function MyComponent( props ) { _Parameters_ - _condition_ `boolean`: Whether the warning will be triggered or not. -- _messages_ `Array`: Message(s) to show in the warning. +- _message_ `string`: Message to show in the warning. diff --git a/packages/warning/src/index.js b/packages/warning/src/index.js index 34ac519f5f741c..9774020697b7a4 100644 --- a/packages/warning/src/index.js +++ b/packages/warning/src/index.js @@ -7,10 +7,10 @@ function isDev() { } /** - * Shows a warning with `messages` if `condition` passes and environment is not `production`. + * Shows a warning with `message` if `condition` passes and environment is not `production`. * * @param {boolean} condition Whether the warning will be triggered or not. - * @param {string[]} messages Message(s) to show in the warning. + * @param {string} message Message to show in the warning. * * @example * ```js @@ -22,7 +22,7 @@ function isDev() { * } * ``` */ -export default function warning( condition, ...messages ) { +export default function warning( condition, message ) { if ( ! isDev() ) { return; } @@ -31,16 +31,14 @@ export default function warning( condition, ...messages ) { return; } - const text = messages.join( '\n' ); - // eslint-disable-next-line no-console - console.warn( text ); + console.warn( message ); // Throwing an error and catching it immediately to improve debugging // A consumer can use 'pause on caught exceptions' // https://github.com/facebook/react/issues/4216 try { - throw Error( text ); + throw Error( message ); } catch ( x ) { // do nothing } diff --git a/packages/warning/src/test/index.js b/packages/warning/src/test/index.js index a2318eb56ab591..b5e83253ab6086 100644 --- a/packages/warning/src/test/index.js +++ b/packages/warning/src/test/index.js @@ -12,19 +12,19 @@ describe( 'warning', () => { it( 'logs to console.warn when NODE_ENV is not "production"', () => { process.env.NODE_ENV = 'development'; - warning( true, 'warn', 'ing' ); - expect( console ).toHaveWarnedWith( 'warn\ning' ); + warning( true, 'warning' ); + expect( console ).toHaveWarnedWith( 'warning' ); } ); it( 'does not log to console.warn if NODE_ENV is "production"', () => { process.env.NODE_ENV = 'production'; - warning( true, 'warn', 'ing' ); + warning( true, 'warning' ); expect( console ).not.toHaveWarned(); } ); it( 'does not log to console.warn if condition is falsy', () => { process.env.NODE_ENV = 'development'; - warning( false, 'warn', 'ing' ); + warning( false, 'warning' ); expect( console ).not.toHaveWarned(); } ); } );