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

Create @babel/plugin-proposal-dynamic-import #9552

Merged
merged 5 commits into from Jun 30, 2019

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Feb 20, 2019

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

This is an implementation of what I proposed in #9551. I had two goals:

  1. It shouldn't be possible, for example, to transform dynamic imports using commonjs and static imports using amd
  2. Since dynamic import is still a proposal, it needs not to be enabled by default by the module transform plugins

Note: sysntemjs was already supported and enabled by default, but this PR merged it's entry point with the one for commonjs/amd.

#4986 is similar, but it only implements CommonJS support and introduces a spec: true mode for commonjs, using different helpers.

@nicolo-ribaudo nicolo-ribaudo added PR: New Feature 🚀 A type of pull request used for our changelog categories Spec: Dynamic Import labels Feb 20, 2019
@babel-bot
Copy link
Collaborator

babel-bot commented Feb 20, 2019

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/10974/

@nicolo-ribaudo nicolo-ribaudo force-pushed the dynamic-import branch 2 times, most recently from 223c1c3 to 33ba9d9 Compare May 7, 2019 23:35
@nicolo-ribaudo nicolo-ribaudo added this to the v7.5.0 milestone May 11, 2019
@nicolo-ribaudo nicolo-ribaudo force-pushed the dynamic-import branch 3 times, most recently from 5d543c1 to 3a1c335 Compare May 13, 2019 19:22
export default declare((api, options) => {
api.assertVersion(7);

// TODO: expose a better interface
const transformImportCall = Function.call.bind(
babelPluginDynamicImportNode(api).visitor.Import,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ignore the ugliness of this line. If this PR gets approved, I will open a PR in the babel-plugin-dynamic-import-node repo to directly export the transformImportCall function.

@xtuc
Copy link
Member

xtuc commented Jun 4, 2019

As we did for ESM, we should force to preserve the import function when using Babel loader in webpack (once it's in babel/preset-env).

define(["require"], function (_require) {
"use strict";

var modP = new Promise(_resolve => _require(["mod"], imported => _resolve(babelHelpers.interopRequireWildcard(imported))));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any case where AMD will synchronously execute "mod" if the code for "mod" is already cached, or is it always async?

If so, the AMD require needs to be moved to a .then to ensure evaluation is asynchronous.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the require source, and apparentely the callback is always called after a context.nextTick:
https://github.com/requirejs/requirejs/blob/fdf4186d3e68df06a04bd71cb6ea0f24eb1600d1/require.js#L1460

It is always guaranteed to be async: requirejs/requirejs#738 (comment)

Reading RequireJS's source code made me realize that it can also generate an error.

const MISSING_PLUGIN_WARNING = `\
WARNING: Dynamic import() transformation must be enabled using the
@babel/plugin-proposal-dynamic-import plugin. As of Babel 8,
without that plugin it won't be transformed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Babel 8 will no longer transform import() without using that plugin."

@nicolo-ribaudo
Copy link
Member Author

Updated to use airbnb/babel-plugin-dynamic-import-node#75

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: New Feature 🚀 A type of pull request used for our changelog categories Spec: Dynamic Import
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic import transform
4 participants