Tree shaking is hindered by dependencies

[denisw]

I just checked why tree shaking is not working correctly as mentioned by @markerikson in his tweet. I tested using the following small example project.

package.json

{
  "name": "example",
  "private": true,
  "scripts": {
    "build": "webpack"
  },
  "dependencies": {
    "redux-starter-kit": ".."
  },
  "devDependencies": {
    "webpack": "^4.28.4",
    "webpack-cli": "^3.2.1"
  },
  "sideEffects": false
}

webpack.config.js

module.exports = {
  mode: 'production'
}

src/index.js

import { createAction } from 'redux-starter-kit'

const foo = createAction('foo')
console.log(foo(123))

When building, all of redux-starter-kit ends up in the output. Looking more at the Webpack tree-shaking optimization, I eventually found the Debugging Optimization Bailouts section, which suggests running Webpack with --display-optimization-bailout to find out when optimizations don't get applied. This is what I got:

> example@ build /Users/denis/Projects/redux-starter-kit/example
> webpack --display-optimization-bailout

Hash: 6728bcf74b0489ce1cf3
Version: webpack 4.28.4
Time: 390ms
Built at: 01/11/2019 10:56:01 PM
  Asset      Size  Chunks             Chunk Names
main.js  23.9 KiB       0  [emitted]  main
Entrypoint main = main.js
 [8] (webpack)/buildin/global.js 472 bytes {0} [built]
     ModuleConcatenation bailout: Module is not an ECMAScript module
 [9] (webpack)/buildin/harmony-module.js 573 bytes {0} [built]
     ModuleConcatenation bailout: Module is not an ECMAScript module
[14] ./src/index.js + 2 modules 13.4 KiB {0} [built]
     ModuleConcatenation bailout: Cannot concat with ../node_modules/immer/dist/immer.module.js (<- Module uses injected variables (process))
     ModuleConcatenation bailout: Cannot concat with ../node_modules/redux-devtools-extension/index.js (<- Module is not an ECMAScript module)
     ModuleConcatenation bailout: Cannot concat with ../node_modules/redux-immutable-state-invariant/dist/index.js (<- Module is not an ECMAScript module)
     ModuleConcatenation bailout: Cannot concat with ../node_modules/redux/es/redux.js (<- Module is referenced from these modules with unsupported syntax: ../node_modules/redux-devtools-extension/index.js (referenced with cjs require))
     | ./src/index.js 104 bytes [built]
     |     ModuleConcatenation bailout: Module is an entry point
     | ../dist/redux-starter-kit.esm.js 12.8 KiB [built]
     |     + 1 hidden module
    + 25 hidden modules

As ModuleConcatenationPlugin is needed for tree-shaking, this suggests that some of the dependencies using require() prevent tree-shaking from happening.

[markerikson]

Tagging #55 as a link.

[markerikson]

The issue isn't immediately critical, I guess, but it'd be nice if we can figure it out.

Related to this, I've been trying to conditionally import redux-immutable-state-invariant in dev only so that it doesn't get pulled in to a production app build. I think I've got it working.

[denisw]

So I went through these module concat errors one-by-one. Here is what I found:

• immer (Module uses injected variables (process)): This is because the immer source code has process.env checks. Apparently this will fix itself with Webpack 5, which can concatenate (and hence tree-shake) in the presence of process.env.

• redux-devtools-extension (Module is not an ECMAScript module): There is simply no module version of this. I guess someone would need to contribute the necessary build system changes (they use Webpack and on top of that Gulp).

• redux-immutable-state-invariant (Module is not an ECMAScript module): Same, no module version. Their current build system is to just process all source files with Babel CLI and output the resulting CommonJS module files.

• redux (Module is referenced from these modules with unsupported syntax: ../node_modules/redux-devtools-extension/index.js (referenced with cjs require): Follows from the redux-devtools-extension problem. As that's CommonJS, it pulls in all of Redux. The extension (and most apps) will need all of Redux anyway, though, so maybe not much of an issue.

[markerikson]

Potentially useful: https://github.com/TrySound/rollup-plugin-size-snapshot

[phryneas]

I just experimented with this a little bit, and I stumbled at redux-devtools-extension: This makes up 6.7kb of the 24-kb-build, and in most cases is absolutely unneccessary, as we could be using window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ instead.

I guess there are still use-cases for the module (electron apps?), but including it by default takes up waaaay too much space IMHO.

@markerikson what do you think - would removing that and using the extension-provided global variable instead be an option?
Alternatively, we could include the module in the development build and the global in the production build?

[markerikson]

That doesn't sound right. Bundlephobia says it's like 600 bytes:

https://bundlephobia.com/result?p=redux-devtools-extension@2.13.8

Ditto for the index file:

https://unpkg.com/browse/redux-devtools-extension@2.13.8/index.js

How are you seeing 6.7K?

And here's the Bundlephobia report for RSK 1.0.1:

https://bundlephobia.com/result?p=redux-starter-kit@1.0.1

11.5K Immer, 6.65K Redux, 5K self.

[phryneas]

In building the project posted by @denisw above twice, once with redux-devtools-extension and once without.

Here are the results:
-rw-r--r-- 1 weber weber 24653 Nov 4 06:47 main.js_1.0.1.js
-rw-r--r-- 1 weber weber 17840 Nov 3 19:09 main.js_without_devtools.js

(second build based on this branch: https://github.com/phryneas/redux-starter-kit/tree/tree-shaking )

[markerikson]

Huh. That seems... odd.

We'll have to dig into this further.

[phryneas]

Okay, a little experimentation later:
The example above uses only createAction, which does not reference redux itself.
So redux could be tree-shaken (the 6.7kb improvement I saw above). But composeWithDevTools is imported from redux-devtools-extension, which is not marked as side-effect-free. So there could be something happening there with redux - because redux is a dependency of redux-devtools-extension. Thus, webpack does not reduce redux from the bundle.

[markerikson]

Ah, interesting.

Now, realistically, I'd expect this to be a moot point because most apps would be using configureStore, which does reference createStore from Redux.

Still, it'd be interesting to figure out what things could and should be tree-shaken, both from RSK and our deps.

[phryneas]

Yup, but it also depends on how much redux could be tree-shaken again.
With redux-devtools-extension in the loop, webpack has to assume that everything in redux is being used. Without it, the code of RSK can be analyzed and just the methods of redux actually being used by RSK can be included.

Another point here is immer - it's being included 100% even if it isn't called at all (when just using createAction). Not sure why though.

[markerikson]

Just filed immerjs/immer#500 to see if they can maybe improve Immer's behavior by adding sideEffects: false and fixing the process reference or something.

[markerikson]

Michel Weststrate is doing some work on improving Immer's tree-shaking, and just tweeted that the Rollup --preserveModules flag seems to be a minimum requirement to get any actual improvement:

https://twitter.com/mweststrate/status/1228056670447730688

[markerikson]

And he also dropped this neat-looking tool:

https://github.com/mweststrate/import-size

[markerikson]

This will be resolved once 1.3 comes out, so closing.