Skip to content

Commit

Permalink
Rewrite sideEffects flags to use only positive patterns (#26452)
Browse files Browse the repository at this point in the history
* Rewrite sideEffects flags to use only positive patterns

* Add sideEffects declarations for src/**/*.scss files
  • Loading branch information
jsnajdr authored Oct 27, 2020
1 parent b6638b2 commit 325498e
Show file tree
Hide file tree
Showing 18 changed files with 20 additions and 61 deletions.
13 changes: 0 additions & 13 deletions packages/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -308,17 +308,4 @@ If your package includes a few files with side effects, you can list them instea
}
```

Many `@wordpress` UI-focused packages rely on side effects for registering blocks, plugins, and data stores. To reduce maintenance costs, it may be preferable to opt for an inverse glob strategy, where you instead list the paths where side effects are *not* present, leaving the bundler to assume that everything else might have them. This results in a glob with multiple roots (to match `@wordpress` package structure) and one or more excluded directories.

Here is an example where we declare that the `components` and `utils` directories are side effect-free:

```json
{
"name": "package",
"sideEffects": [
"!((src|build|build-module)/(components|utils)/**)"
],
}
```

Please consult the [side effects documentation](./side-effects.md) for more information on identifying and declaring side effects.
4 changes: 0 additions & 4 deletions packages/block-directory/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@
"main": "build/index.js",
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"build-style/**",
"!((src|build|build-module)/components/**)"
],
"dependencies": {
"@wordpress/a11y": "file:../a11y",
"@wordpress/api-fetch": "file:../api-fetch",
Expand Down
3 changes: 2 additions & 1 deletion packages/block-editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
"react-native": "src/index",
"sideEffects": [
"build-style/**",
"!((src|build|build-module)/(components|utils)/**)"
"src/**/*.scss",
"{src,build,build-module}/{index.js,store/index.js,hooks/**}"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"build-style/**"
"build-style/**",
"src/**/*.scss"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/blocks/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"!((src|build|build-module)/api/**)"
"{src,build,build-module}/{index.js,store/index.js}"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
Expand Down
3 changes: 2 additions & 1 deletion packages/components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"build-style/**"
"build-style/**",
"src/**/*.scss"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/core-data/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"(src|build|build-module)/index.js"
"{src,build,build-module}/index.js"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
Expand Down
4 changes: 0 additions & 4 deletions packages/edit-navigation/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@
"main": "build/index.js",
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"build-style/**",
"!((src|build|build-module)/components/**)"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
"@wordpress/api-fetch": "file:../api-fetch",
Expand Down
4 changes: 0 additions & 4 deletions packages/edit-post/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@
"main": "build/index.js",
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"build-style/**",
"!((src|build|build-module)/(components|utils)/**)"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
"@wordpress/a11y": "file:../a11y",
Expand Down
4 changes: 0 additions & 4 deletions packages/edit-site/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@
"main": "build/index.js",
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"build-style/**",
"!((src|build|build-module)/components/**)"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
"@wordpress/a11y": "file:../a11y",
Expand Down
4 changes: 0 additions & 4 deletions packages/edit-widgets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@
"main": "build/index.js",
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"build-style/**",
"!((src|build|build-module)/components/**)"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
"@wordpress/api-fetch": "file:../api-fetch",
Expand Down
3 changes: 2 additions & 1 deletion packages/editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"react-native": "src/index",
"sideEffects": [
"build-style/**",
"!((src|build|build-module)/(components|utils)/**)"
"src/**/*.scss",
"{src,build,build-module}/{index.js,store/index.js,hooks/**}"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
Expand Down
3 changes: 2 additions & 1 deletion packages/interface/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
"react-native": "src/index",
"sideEffects": [
"build-style/**",
"!((src|build|build-module)/components/**)"
"src/**/*.scss",
"{src,build,build-module}/{index.js,store/index.js}"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
Expand Down
3 changes: 2 additions & 1 deletion packages/nux/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"react-native": "src/index",
"sideEffects": [
"build-style/**",
"!((src|build|build-module)/components/**)"
"src/**/*.scss",
"{src,build,build-module}/{index.js,store/index.js}"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
Expand Down
4 changes: 3 additions & 1 deletion packages/primitives/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
"main": "build/index.js",
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": false,
"sideEffects": [
"src/**/*.scss"
],
"types": "build-types",
"dependencies": {
"@babel/runtime": "^7.11.2",
Expand Down
3 changes: 1 addition & 2 deletions packages/reusable-blocks/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"build-style/**",
"!((src|build|build-module)/(components|utils)/**)"
"{src,build,build-module}/{index.js,store/index.js}"
],
"dependencies": {
"@wordpress/block-editor": "file:../block-editor",
Expand Down
3 changes: 2 additions & 1 deletion packages/rich-text/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": [
"!((src|build|build-module)/component/**)"
"src/**/*.scss",
"{src,build,build-module}/{index.js,store/index.js}"
],
"dependencies": {
"@babel/runtime": "^7.11.2",
Expand Down
16 changes: 0 additions & 16 deletions packages/side-effects.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,3 @@ If it has a few files with side effects, it can list them:
```

This allows the bundler to assume that only the modules that were declared have side effects, and *nothing else does*. Of course, this means that we need to be careful to include everything that *does* have side effects, or problems can arise in applications that make use of the package.

## The approach in `@wordpress`

In order to reduce maintenance cost and minimize the chance of breakage, we opted for using inverse globs for a number of `@wordpress` packages, where we list the paths that *do not* include side effects, leaving the bundler to assume that everything else does. Here's an example:

```json
{
"sideEffects": [
"!((src|build|build-module)/(components|utils)/**)"
],
}
```

The above means that the bundler should assume that anything outside the `components` and `utils` directories contains side effects, and nothing in those directories does. These directories can be inside of a `src`, `build`, or `build-module` top-level directory in the package, due to the way `@wordpress` packages are built.

This approach should guarantee that everything in `components` and `utils` can be tree-shaken. It will only potentially cause problems if one of the files in there uses side effects, which would be a bad practice for a component or utility file.

0 comments on commit 325498e

Please sign in to comment.