-
Notifications
You must be signed in to change notification settings - Fork 33
Add babel helpers and support for named chunks. #30
base: master
Are you sure you want to change the base?
Conversation
Webpack 2 added support for named chunks when using `import()` via a preceding comment. This adds the same support to this babel plugin. webpack/webpack#4573
@@ -1,5 +1,7 @@ | |||
function _interopRequireWildcard(obj) { if (obj && obj.__esModule) { return obj; } else { var newObj = {}; if (obj != null) { for (var key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) newObj[key] = obj[key]; } } newObj.default = obj; return newObj; } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be determined by the user's setting to use babelHelpers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
babelHelpers are included either way. User's have the option of using external-babel-helpers
in which case babel will take care of not inlining this, but pulling it from wherever the user provides it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah ok, so if we're using external-babel-helpers
, then this helper wouldn't appear?
In that case, could we add some parallel tests that use that option, so we can ensure that the helpers appear or not following that setting?
@ljharb Added tests for applying the plugin w/ external-helpers. |
@@ -3,22 +3,54 @@ import syntax from 'babel-plugin-syntax-dynamic-import'; | |||
|
|||
const TYPE_IMPORT = 'Import'; | |||
|
|||
const buildImport = template(` | |||
const buildImport = opts => template(` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it might be nice if we explicitly destructured the template substitutions (and explicitly referenced them on line 12) instead of having it be implicit?
This may be considered breaking; thoughts? |
Hi what is status of this PR? |
@ljharb Yes, this would be considered a breaking change. The babel interop helpers transforms the shape of the imported module if it doesn't include the property @ondrejbartas I let it sit for longer than I should've but am still hoping to get this merged. |
@kesne @ljharb
This changes the plugin to use babelHelpers to return the resulting require rather than directly returning it. This should remain more spec compliant over time. Also, it adds support for named chunks via a preceding comment, following the same style as webpack@2.
If this goes through I'll put up a pr for https://github.com/airbnb/babel-plugin-dynamic-import-node.
This should address the concerns brought up in - #13