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

DevTools Extension package is not being tree-shaken in production #380

Closed
markerikson opened this issue Feb 21, 2020 · 2 comments
Closed
Milestone

Comments

@markerikson
Copy link
Collaborator

markerikson commented Feb 21, 2020

Per #78 , and similar to #363 , the redux-devtools-extension package is not written as ES Modules, and is breaking Webpack's ability to optimize it out of bundles.

Per that discussion, the package is itself tiny, and the fact that it results in all of Redux being pulled in is not likely to cause issues in practice because you're almost definitely using Redux. But, as a general principle, we'd like it to be shakeable.

I did go ahead and file zalmoxisus/redux-devtools-extension#718 to see if we can get it converted to ESM or TS. However, the package has been irregularly maintained for a while, so I don't expect a fast response.

Given that, I've gone ahead and updated the repo fork I'd previously made at https://github.com/reduxjs/redux-devtools-extension , and am currently attempting to convert the package to TypeScript. I've successfully published a first attempt on NPM as @reduxjs/[email protected].

Following the instructions at #78 (comment) , I created a minimal project that only imports createAction and runs it through Webpack. With the existing [email protected], the output was:

      Asset     Size  Chunks                   Chunk Names
    main.js   28 KiB       0  [emitted]        main
main.js.map  154 KiB       0  [emitted] [dev]  main
Entrypoint main = main.js main.js.map
 [7] (webpack)/buildin/global.js 472 bytes {0} [built]
     ModuleConcatenation bailout: Module is not an ECMAScript module
 [8] (webpack)/buildin/harmony-module.js 573 bytes {0} [built]
     ModuleConcatenation bailout: Module is not an ECMAScript module
[14] ./src/index.js + 3 modules 35.1 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/nanoid/index.browser.js (<- Module is not an ECMAScript module)
     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/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 105 bytes [built]
     |     ModuleConcatenation bailout: Module is an entry point
     |     + 3 hidden modules
    + 12 hidden modules

I then modified my local copy of RTK to use @reduxjs/redux-devtools-extension-fork instead, did a local test publish using yalc, and updated the test project to use that instead. Rerunning the same command:

      Asset      Size  Chunks                   Chunk Names
    main.js  21.7 KiB       0  [emitted]        main
main.js.map   148 KiB       0  [emitted] [dev]  main
Entrypoint main = main.js main.js.map
 [5] (webpack)/buildin/global.js 472 bytes {0} [built]
     ModuleConcatenation bailout: Module is not an ECMAScript module
 [6] (webpack)/buildin/harmony-module.js 573 bytes {0} [built]
     ModuleConcatenation bailout: Module is not an ECMAScript module
[12] ./src/index.js + 5 modules 59.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/nanoid/index.browser.js (<- Module is not an ECMAScript module)
     ModuleConcatenation bailout: Cannot concat with ./node_modules/symbol-observable/es/index.js (<- Module uses injected variables (global, module))
     | ./src/index.js 105 bytes [built]
     |     ModuleConcatenation bailout: Module is an entry point
     |     + 5 hidden modules
    + 10 hidden modules

So, main.js shrank from 28K to 21K, indicating that the Redux core was no longer being pulled in, and I can see that there's no warnings about the redux-devtools-extension package.

Given that, this looks like a promising approach, and it's worth trying to see if it works out as part of the alphas.

It's late atm, so I'll try merging things and publishing an alpha with this tomorrow.

The PR for the extension fork is at reduxjs/redux-devtools-extension#2 .

@markerikson markerikson added this to the 1.3 milestone Feb 21, 2020
@markerikson
Copy link
Collaborator Author

On second thought, why bother with a forked package? It's basically one file, so I'm just going to move it in to RTK directly.

@markerikson
Copy link
Collaborator Author

Resolved in the v1.3 alphas by #384 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant