Bump react-shallow-renderer to 16.13.1

[NMinhNguyen]

See webpack/webpack#7973

Most recent edit

I've now removed the module field from react-shallow-renderer to avoid ESM/CJS interop issues and published v16.13.1 of the package (see related PR enzymejs/react-shallow-renderer#34).

Edit

Just came across webpack/webpack#6584 as well

Summary

This change fixes a plausible CJS <-> ESM interop issue in webpack that I mentioned in #18186 (comment) (see webpack/webpack#7973 for details).

I was actually able to reproduce this issue by tweaking node_modules (to use react-shallow-renderer) and running yarn test:karma in Material-UI's repo - https://github.com/mui-org/material-ui/blob/aa0963253bd682d0c08f21114037749e3e9f0817/package.json#L36

Although it does pick up the ESM version of react-shallow-renderer, Enzyme uses _interopRequireDefault (https://unpkg.com/browse/enzyme-adapter-react-16@1.15.2/build/ReactSixteenAdapter.js) when require()-ing it so things don't actually break. But imagine a proper CJS setup (with webpack) where users are doing require('react-test-renderer/shallow'), they could get this .default key.

I'm not so concerned about direct imports of react-shallow-renderer (as in, not transitively via react-test-renderer/shallow) because it's a new package, so I don't think dropping the ESM build is worth it.

Note: this issue doesn't affect Jest (and as a result, CRA).

Test Plan

I modified react-test-renderer/shallow like so in Material-UI's node_modules:

const ReactShallowRenderer = require('react-shallow-renderer')

console.log('Resolved path:', require.resolve('react-shallow-renderer'))
console.log({__esModule: ReactShallowRenderer.__esModule, ReactShallowRenderer})

module.exports = ReactShallowRenderer;

And got the following output:

HeadlessChrome 80.0.3987 (Mac OS X 10.15.1) LOG: 'Resolved path: ./node_modules/react-shallow-renderer/esm/index.js'
HeadlessChrome 80.0.3987 (Mac OS X 10.15.1) LOG: Object{__esModule: true, ReactShallowRenderer: Object{default: function ReactShallowRenderer() { ... }}}

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2a94183:

Sandbox	Source
dank-tree-ld9ho	Configuration

[sizebot]

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 2a94183

[sizebot]

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against 2a94183

[NMinhNguyen]

Alternatively, I could introduce a breaking change to react-shallow-renderer (making it v17.0.0) to only used named exports, and then we tweak react-test-renderer to re-export it accordingly (and without interop workarounds).

Something like

// react-shallow-renderer/src/ReactShallowRenderer.js
- export default ReactShallowRenderer;
+ export { ReactShallowRenderer }; // while still keeping the static `createRenderer` property for backwards compat
+ export const createRenderer = function() {
+   return new ReactShallowRenderer();
+ };

// packages/react-test-renderer/npm/shallow.js
- module.exports = require('react-shallow-renderer');
+ module.exports = require('react-shallow-renderer').ReactShallowRenderer;

// packages/react-test-renderer/shallow.js
- export {default} from 'react-shallow-renderer';
+ export {ReactShallowRenderer as default} from 'react-shallow-renderer';

[gaearon]

Exposing the class was always weird (and probably an accident given createRenderer exists). Unfortunately that's what the docs already show so it's likely people use it widely.

[NMinhNguyen]

Exposing the class was always weird (and probably an accident given createRenderer exists). Unfortunately that's what the docs already show so it's likely people use it widely.

Yeah I don’t think I should remove the class export. However, given it’s a new package (and it’s unlikely that a lot of people have adopted it already), I’m open to cutting a new major release that uses a named export to export the class. When/if people migrate from react-test-renderer/shallow they’d have to update their imports anyway, and in my opinion it’s not that much more work to grab (destructure in CJS) ReactShallowRenderer.

@gaearon has suggested via Twitter DM to remove module field (since the ESM story is somewhat unclear atm) which I’m kinda open to doing as well, but I think I slightly prefer switching to named exports. @sebmarkbage do you have any thoughts on this? Removing module might mean re-enabling commonjs plugin for the shallow renderer (like you’re already doing for ART), but maybe that’s an acceptable trade-off?

[NMinhNguyen]

One more solution: follow the approach taken in mridgway/hoist-non-react-statics#86 to ensure the CJS version always exports a .default property. This would be a breaking change for CJS but ESM would be unaffected.