Make this package tree-shakeable

[Andarist]

I'm building a UI library which exports a bunch of stuff, only some of my components rely on this package so it would be desirable that if a consumer of my package doesn't use component relying on tabbable it would be tree shaken out of the final bundle. However it's not the case right now.

Let's focus on 2 common tree-shaking deopts:

• no ESM bundle (this package) exports only CJS format
• top-level static properties, as in module.exports = tabbable; tabbable.isTabbable = isTabbable;

Action items:

• migrate source to ESM + build CJS format from it, point to CJS from package.json#main and to ESM from package.json#module so bundlers can pick appropriate file
• change exports shape to export default tabbable; export { isTabbable, isFocusable };

Intent to implement - yes (was already implemented in #38 )

[davidtheclark]

I have the impression this might break compatibility with Webpack: webpack/webpack#5756

I'd rather not spend energy getting experimental with the build for this library.

Is there no way your downstream UI library can structure itself to optimize for tree-shaking? For example, if this module were only imported by the UI components that use it — why wouldn't tree-shaking work then, for consumers of the UI library?

[Andarist]

I have the impression this might break compatibility with Webpack: webpack/webpack#5756

It won't if you are going to provide same exports shape for both ESM & CJS files - the compatibility only breaks when people try to use so called auto exports for CJS, so basically transforming export default foo to module.exports = foo. Transform to compliant safe version - module.exports.default = foo is completely safe.

Is there no way your downstream UI library can structure itself to optimize for tree-shaking? For example, if this module were only imported by the UI components that use it — why wouldn't tree-shaking work then, for consumers of the UI library?

Unfortunately bundlers, minifiers & such are quite conservative regarding this stuff. Most of the bundlers simply bail out on CJS files because of its dynamic nature (even though most of the real world files have static exports). In order to perform safe tree shaking a tool has to analyze the program and if it's unsure if a statement has no side effects then it's marked as unsafe to be removed.

I've illustrated this problem some time ago with a simple repro case. Consider a simple structure:

• index.js - importing and using A
• module.js - containing A and B, where only B uses classnames (example of CJS-only library)

classnames is still in the bundle, although it is not used by the "app".

[stefcameron]

Based on discussion in #53, this issue isn't fully addressed yet. We still need to mark the package as having no side-effects, and add the PURE designation to one line. See discussion on #53 for more context.

[stefcameron]

After #59, I believe the only thing left is adding sideEffects: false to package.json and we can close this issue. 🎉

[Andarist]

@stefcameron sideEffects: false is pretty much useless (in case of the webpack) for tabbable. It only works on packages that have non-flat structure and you only provide a single flat bundle. See the demo I've prepared here: https://github.com/Andarist/tabbable-tree-shaking-issue-demo

Even though my entry point only uses stuff unrelated to tabbable the non-treeshakable code in tabbable is still left in the bundle: https://github.com/Andarist/tabbable-tree-shaking-issue-demo/blob/5a2339a20353dce91786c3467048e22cab1e9a6c/dist/main.js#L58-L75

[stefcameron]

@stefcameron sideEffects: false is pretty much useless (in case of the webpack) for tabbable. It only works on packages that have non-flat structure and you only provide a single flat bundle. See the demo I've prepared here: https://github.com/Andarist/tabbable-tree-shaking-issue-demo

Even though my entry point only uses stuff unrelated to tabbable the non-treeshakable code in tabbable is still left in the bundle: https://github.com/Andarist/tabbable-tree-shaking-issue-demo/blob/5a2339a20353dce91786c3467048e22cab1e9a6c/dist/main.js#L58-L75

I see, thanks for pointing that out. Doesn't hurt, but seems ineffective. Thanks for #70