fix: _interopRequireWildcard should only cache objects

[samMeow]

Q	A
Fixed Issues?	Fixes #10561
Patch: Bug Fix?	Yes
Major: Breaking Change?	No
Minor: New Feature?	No
Tests Added + Pass?	Yes
Documentation PR Link	No
Any Dependency Changes?	No
License	MIT

_interopRequireWildcard (used for dynamic import) wrongly cached string modules (e.g. base64 image) to Weakmap throws error. This fix is to cache object only for the function.

[buildsize]

File name	Previous Size	New Size	Change
babel-preset-env.js	2.73 MB	2.73 MB	136 bytes (0%)
babel-preset-env.min.js	1.65 MB	1.65 MB	132 bytes (0%)
babel.js	2.92 MB	2.92 MB	136 bytes (0%)
babel.min.js	1.61 MB	1.61 MB	132 bytes (0%)
test262.tap	4.84 MB	[deleted]

[nicolo-ribaudo]

This isn't safe, for example, if __proto__ is null. You should use typeof.

[JLHwung]

I wish we can use _.isObject in the helpers. I copied from there

typeof obj === "object" || typeof obj === "function"

because (new WeakMap).set(new Function, 42) is valid.

[samMeow]

Thanks for the advices. Inspiring from _.isObject. We should also add null check, because typeof null === 'object' and (new WeakMap).set(null, { default: null }) throws error.

Nicolò points out a potential issue.

[theKashey]

What about adding tests for this? It would be easier to understand how this function should work, and prevent possible issues in the future.

[samMeow]

For test case, you may refer to packages/babel-plugin-proposal-dynamic-import/test/fixtures/commonjs/*. You will find an example for function module exec-interop and newly added exec-interop-string, exec-interop-null.

[theKashey]

Sorry for the offtopic, but are you sure it's still working correctly?

For example, lets double check construction like import * as classnames from 'classnames'
classnames are defined in a more or less common way, for many "old" packages

classNames.default = classNames;
module.exports = classNames;

So let's imagine - import * as classnames from 'classnames' (* required by TS, as long as there is no(properly defined) "default" import)

• Q: What webpack would emit:

• A: var classnames__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! classnames */ "./node_modules/classnames/index.js");

• Q: What is it?

• A: It's a function you can use

• Q: What would happen next?

• A Nothing bad

---

• Q: What parcel would emit (uses babel):

• A: var classnames = _interopRequireWildcard(require("classnames"));

• Q: What is it?

• A: It's a {default: classNames}

• Q: What would happen next?

• A: classnames is not a function.

---

• Q: What would happen if import classnames from 'classnames'?
• A: It would work, but that's not correct.

So, question - what _interopRequireWildcard is doing? Why it's trying to return an object with .default

As far as I understand - the inner loop creates an object clone, plus stores the original object as default. Look like that's working for objects, but not for the functions.

So:

• _interopRequireWildcard(1) == 1
• _interopRequireWildcard(false) == false
• _interopRequireWildcard("hey") == "hey
• _interopRequireWildcard({test: 42}).test == 42
• _interopRequireWildcard(function(){return 'test')) === ???

Right now compiling the same file using webpack or parcel(babel) produces not only different code - it's not working in the second case.

[nicolo-ribaudo]

import * as x must always return an object. If webpack returns a function, it's a webpack bug. (To be 100% correct, it should be an object with __proto__ === null and read-only)

In your example, import classnames from 'classnames' is correct.

[theKashey]

Is it a part of ESM specification? As long as that would be TypeScript bug in this case, and all the projects using TypeScript and wildcard imports to read exports as is from commonjs module, and exposing ESM modules as the output... are broken.
Well, this is how I got into this issue - ‘import * as isNode’ from “detect-node”’ - and no default export - https://unpkg.com/browse/@types/detect-node@2.0.0/index.d.ts

[nicolo-ribaudo]

@theKashey Yes, the specification defines imported namespaces (with * as foo) as that kind of exotic objects. The default exported by a module is accessible as foo.default.
That object is defined at https://tc39.es/ecma262/#table-29. As you can see at https://tc39.es/ecma262/#function-object, a function is an object with an internal [[Call]] property, which imported namespaces don't have.

Also note that Babel aligns with Node's implementation in this case: the default export of a module is it's module.exports.

[ExE-Boss]

There’s a second non‑nullish check a few lines after this, so I’m removing it in #10585, since it’s no longer necessary.