fix: workaround for weirdly packaged cjs modules #1178
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
We found a weird bug when shaker attempted to operate on preact/compat. Note that it's probably a bug that shaker is even processing this file, since it means we didn't strip the import in an earlier stage, but I'm not sure what's going on there.
I think the issue is demonstrated pretty clearly in the unit test, but the gist is that certain libraries might be packaged in a way where they have a
default
export, but no__esModule: true
export. This will break babel's interopRequireDefault helper, which wraps its argument if it doesn't detect__esModule: true
, resulting in an object like{ default: { default: someExport } }
.In the case of preact/compat, this is fine, because all of the exports are also defined directly on
module.exports
. resulting in{ default: { default: someExport, someExport } }
. So even if interopRequireDefault produces a broken result,interopRequireDefault(...).default.someExport
still exists.Thee trouble comes when shaker deletes
someExport
because it was not inonlyExports
, leaving only the default export; now, interopRequireDefault produces{ default: { default: someExport } }
andinteropRequireDefault(...).default.someExport
will be undefined.Summary
I added detection of __esModule to collectImportAndExports. Now, if we detect that
default
is inonlyExports
, and the module is not an esModule, we'll skip the shaker process as it will be unsafe.Test plan
Added a unit test.