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

Improve dynamic import interop #2954

Merged
merged 5 commits into from Jun 21, 2019
Merged

Conversation

lukastaegert
Copy link
Member

@lukastaegert lukastaegert commented Jun 21, 2019

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:
Resolves #2949

Description

This will improve CJS and AMD interop in several points:

  • When a chunk dynamically imports an entry chunk that only contains a default export, the imported namespace will now correctly have the form {default: myVariable} instead of being directly myVariable

  • When dynamically importing an external module, AMD and CJS will now contain a new interop helper that makes sure that there is a default export and that live-bindings work. The helper has the following form:

    function _interopNamespace(e) {
      if (e && e.__esModule) { return e; } else {
      	var n = {};
      	if (e) {
      		Object.keys(e).forEach(function (k) {
      			var d = Object.getOwnPropertyDescriptor(e, k);
      			Object.defineProperty(n, k, d.get ? d : {
      				enumerable: true,
      				get: function () {
      					return e[k];
      				}
      			});
      		});
      	}
      	n['default'] = e;
      	return n;
      }
    }

    Some explanation:

    • When the __esModule prop is present on the exports e, we know this is a namespace and reuse it
    • If it is falsy, just return an object {default: e}
    • Otherwise first recreate all own enumerable properties. If those properties are getters, just copy the property descriptor, otherwise we create our own getter that evaluates the property on access to allow live bindings (existing getters are copied to avoid cascaded getters when doing this over multiple exports). Then we attach the exports to the default property.

    This is similar to what Babel does except that it maintains correct live-bindings in more situations.

  • When we export * from ..., then the default is not reexported (which is the correct behaviour).

Possible future improvement: Currently, the new helper is not deduplicated, i.e. each chunk gets its own helper. If we want to have the helper only once in the graph, it would be good to move the chunking into the generate phase first (which I think is a very good idea) as then we will have format dependent chunks.

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

Successfully merging this pull request may close these issues.

Dynamic import of third party cjs modules does not resolve default exports
1 participant