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

babel-plugin-transform-runtime #6983

Closed
jdalton opened this Issue Dec 6, 2017 · 17 comments

Comments

Projects
None yet
4 participants
@jdalton
Member

jdalton commented Dec 6, 2017

Using @babel/plugin-transform-runtime of ^7.0.0-beta.34 I noticed that it doesn't generate _interopRequireDefault with useESModules:true and .js files

The code is something like:

var _wrapAsyncGenerator = require("@babel/runtime/helpers/builtin/es6/wrapAsyncGenerator");

However, if I rename my file from .js to .mjs then it produces the helper

var _interopRequireDefault = require("@babel/runtime/helpers/builtin/es6/interopRequireDefault");
var _wrapAsyncGenerator2 = _interopRequireDefault(require("@babel/runtime/helpers/builtin/es6/wrapAsyncGenerator"));

I would expect the helper to exist for either file extension.

@babel-bot

This comment has been minimized.

Show comment
Hide comment
@babel-bot

babel-bot Dec 6, 2017

Collaborator

Hey @jdalton! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

Collaborator

babel-bot commented Dec 6, 2017

Hey @jdalton! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Dec 6, 2017

Member

I also noticed that with the .mjs file extension the _interopRequireDefault should be used on itself

var _interopRequireDefault = require("@babel/runtime/helpers/builtin/es6/interopRequireDefault");

should be

var _interopRequireDefault = require("@babel/runtime/helpers/builtin/es6/interopRequireDefault");
_interopRequireDefault = _interopRequireDefault(_interopRequireDefault)
Member

jdalton commented Dec 6, 2017

I also noticed that with the .mjs file extension the _interopRequireDefault should be used on itself

var _interopRequireDefault = require("@babel/runtime/helpers/builtin/es6/interopRequireDefault");

should be

var _interopRequireDefault = require("@babel/runtime/helpers/builtin/es6/interopRequireDefault");
_interopRequireDefault = _interopRequireDefault(_interopRequireDefault)
@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Dec 6, 2017

Member

If I set "modules":false with "useESModules":true the .mjs will generate static imports like

import _wrapAsyncGenerator from "@babel/runtime/helpers/builtin/es6/wrapAsyncGenerator";

where as the .js file will generate require calls like

var _wrapAsyncGenerator = require("@babel/runtime/helpers/builtin/es6/wrapAsyncGenerator");

The "sourceType":"module" or "sourceType":"unambiguous" doesn't resolve this since the file source is:

async function* gen() {
  yield 'first'
  yield 'last'
}

async function main() {
  for await (let item of gen()) {
    console.log(item)
  }
}

main()

and has no import or export.

If I add an export {} then either sourceType of "module" or "unambiguous" seem to generate static imports.

Member

jdalton commented Dec 6, 2017

If I set "modules":false with "useESModules":true the .mjs will generate static imports like

import _wrapAsyncGenerator from "@babel/runtime/helpers/builtin/es6/wrapAsyncGenerator";

where as the .js file will generate require calls like

var _wrapAsyncGenerator = require("@babel/runtime/helpers/builtin/es6/wrapAsyncGenerator");

The "sourceType":"module" or "sourceType":"unambiguous" doesn't resolve this since the file source is:

async function* gen() {
  yield 'first'
  yield 'last'
}

async function main() {
  for await (let item of gen()) {
    console.log(item)
  }
}

main()

and has no import or export.

If I add an export {} then either sourceType of "module" or "unambiguous" seem to generate static imports.

@nicolo-ribaudo

This comment has been minimized.

Show comment
Hide comment
@nicolo-ribaudo

nicolo-ribaudo Dec 6, 2017

Member

⚠️ I just noticed I might be wrong, I need to check


_interopRequireDefault isn't needed when requireing helpers because they are guaranteed to be generated from es modules.

This is the _interopRequireDefault definition:

function _interopRequireDefault(obj) {
  return obj && obj.__esModule ? obj : { default: obj };
}

In babel helpers, __esModule is always true so that function is equivalent to the identity function, which can be elided.

When you use the .mjs extension, Babel injectsimport _wrapAsyncGenerator from "@babel/runtime/.... Then the modules-to-commonjs plugin runs, sees a default import and transforms it like it does for any default import: var NAME = _interopRequireDefault(require("SOURCE")).

Regarding your last comment, babel inserts require calls instead of import statements (which would then be transpiled to require calls with interopRequireDefault) because if the extension is .js and the source code can be parsed as a script (it doesn't contain export/import), it inserts require calls for backwards compatibility. You can see the logic used to choose between require and import here.

Member

nicolo-ribaudo commented Dec 6, 2017

⚠️ I just noticed I might be wrong, I need to check


_interopRequireDefault isn't needed when requireing helpers because they are guaranteed to be generated from es modules.

This is the _interopRequireDefault definition:

function _interopRequireDefault(obj) {
  return obj && obj.__esModule ? obj : { default: obj };
}

In babel helpers, __esModule is always true so that function is equivalent to the identity function, which can be elided.

When you use the .mjs extension, Babel injectsimport _wrapAsyncGenerator from "@babel/runtime/.... Then the modules-to-commonjs plugin runs, sees a default import and transforms it like it does for any default import: var NAME = _interopRequireDefault(require("SOURCE")).

Regarding your last comment, babel inserts require calls instead of import statements (which would then be transpiled to require calls with interopRequireDefault) because if the extension is .js and the source code can be parsed as a script (it doesn't contain export/import), it inserts require calls for backwards compatibility. You can see the logic used to choose between require and import here.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Dec 6, 2017

Member

@nicolo-ribaudo

With "modules":false and useESModules:true there is a state where the require() helper is ESM via require("@babel/runtime/helpers/builtin/es6/wrapAsyncGenerator") . This normally won't work, but with @std/esm it can (kindof). Except that it would expect the result of require of the ESM code to be { default: wrapAsyncGeneratorFunc }. So I use a compat mode which will load it with __esModule on its module.export which is normally handled by the interopRequireDefault helper but since the interopRequireDefault doesn't check itself then it's not helped 😋

Member

jdalton commented Dec 6, 2017

@nicolo-ribaudo

With "modules":false and useESModules:true there is a state where the require() helper is ESM via require("@babel/runtime/helpers/builtin/es6/wrapAsyncGenerator") . This normally won't work, but with @std/esm it can (kindof). Except that it would expect the result of require of the ESM code to be { default: wrapAsyncGeneratorFunc }. So I use a compat mode which will load it with __esModule on its module.export which is normally handled by the interopRequireDefault helper but since the interopRequireDefault doesn't check itself then it's not helped 😋

@nicolo-ribaudo

This comment has been minimized.

Show comment
Hide comment
@nicolo-ribaudo

nicolo-ribaudo Dec 6, 2017

Member

Oh, I get the problem now. So if useESModules: true, babel should always inject imports and then transpile them.

Member

nicolo-ribaudo commented Dec 6, 2017

Oh, I get the problem now. So if useESModules: true, babel should always inject imports and then transpile them.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Dec 6, 2017

Member

@nicolo-ribaudo

Oh, I get the problem now. So if useESModules: true, babel should always inject imports and then transpile them.

That might be the easier way. Ya.

Member

jdalton commented Dec 6, 2017

@nicolo-ribaudo

Oh, I get the problem now. So if useESModules: true, babel should always inject imports and then transpile them.

That might be the easier way. Ya.

@nicolo-ribaudo

This comment has been minimized.

Show comment
Hide comment
@nicolo-ribaudo

nicolo-ribaudo Dec 6, 2017

Member

Or it could output something smaller and easier like

var _awaitAsyncGenerator = require("@babel/runtime/helpers/es6/awaitAsyncGenerator").default;
var _wrapAsyncGenerator = require("@babel/runtime/helpers/es6/wrapAsyncGenerator").default;

let foo =
/*#__PURE__*/
(() => {
  var _ref = _wrapAsyncGenerator(function* () {});

  return function foo() {
    return _ref.apply(this, arguments);
  };
})();

This is possible only because we know that helpers' default export is never reassigned, so it doesn't need to be a live binding.

Member

nicolo-ribaudo commented Dec 6, 2017

Or it could output something smaller and easier like

var _awaitAsyncGenerator = require("@babel/runtime/helpers/es6/awaitAsyncGenerator").default;
var _wrapAsyncGenerator = require("@babel/runtime/helpers/es6/wrapAsyncGenerator").default;

let foo =
/*#__PURE__*/
(() => {
  var _ref = _wrapAsyncGenerator(function* () {});

  return function foo() {
    return _ref.apply(this, arguments);
  };
})();

This is possible only because we know that helpers' default export is never reassigned, so it doesn't need to be a live binding.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Dec 6, 2017

Member

@nicolo-ribaudo

Yap, that would work too! 🤘

Member

jdalton commented Dec 6, 2017

@nicolo-ribaudo

Yap, that would work too! 🤘

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Dec 6, 2017

Member

To me this seems like expected behavior of using unambiguous grammar. I think I would have assumed that std/esm would add export {}; to force processing as a ES module if that's what you want.

Member

loganfsmyth commented Dec 6, 2017

To me this seems like expected behavior of using unambiguous grammar. I think I would have assumed that std/esm would add export {}; to force processing as a ES module if that's what you want.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Dec 7, 2017

Member

@loganfsmyth

For the comment about Babel generating require calls for .js files with "modules":false and "useESModules":true instead of static imports, I found it odd that specifying sourceType of "module" didn't get Babel to generate static imports either.

Member

jdalton commented Dec 7, 2017

@loganfsmyth

For the comment about Babel generating require calls for .js files with "modules":false and "useESModules":true instead of static imports, I found it odd that specifying sourceType of "module" didn't get Babel to generate static imports either.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Dec 7, 2017

Member

Ah of course. You are right about sourceType:module. We added unambiguous logic to Babylon but haven't removed the logic from the module transform yet. If we remove that it should fix this.

We should also decide if we want to default to script for .js files. I feel like we should probably?

Member

loganfsmyth commented Dec 7, 2017

Ah of course. You are right about sourceType:module. We added unambiguous logic to Babylon but haven't removed the logic from the module transform yet. If we remove that it should fix this.

We should also decide if we want to default to script for .js files. I feel like we should probably?

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Dec 7, 2017

Member

@loganfsmyth

We should also decide if we want to default to script for .js files.

Doesn't Babel just default to "module" (which is good to me) for everything?

Member

jdalton commented Dec 7, 2017

@loganfsmyth

We should also decide if we want to default to script for .js files.

Doesn't Babel just default to "module" (which is good to me) for everything?

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Dec 7, 2017

Member

It does default to module, but that means people trip over it whenever they want to compile things in node_modules since we inject use strict on things that aren't strict and rewrite this to undefined. Defaulting to script would be way better for consistency.

Member

loganfsmyth commented Dec 7, 2017

It does default to module, but that means people trip over it whenever they want to compile things in node_modules since we inject use strict on things that aren't strict and rewrite this to undefined. Defaulting to script would be way better for consistency.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Dec 7, 2017

Member

It does default to module, but that means people trip over it whenever they want to compile things in node_modules since we inject use strict on things that aren't strict and rewrite this to undefined.

Ah, I thought that's what "unambiguous" was created to avoid. Would defaulting to "unambiguous" make sense? It's what TypeScript does I believe.

....But ...Maybe the "What should the default sourceType be" rehash should be a separate issue. Feel free to cc me on it too please.

Member

jdalton commented Dec 7, 2017

It does default to module, but that means people trip over it whenever they want to compile things in node_modules since we inject use strict on things that aren't strict and rewrite this to undefined.

Ah, I thought that's what "unambiguous" was created to avoid. Would defaulting to "unambiguous" make sense? It's what TypeScript does I believe.

....But ...Maybe the "What should the default sourceType be" rehash should be a separate issue. Feel free to cc me on it too please.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Dec 7, 2017

Member

Yeah fine with that being separate, though we may already have one for that. Can't look at the moment though.

Member

loganfsmyth commented Dec 7, 2017

Yeah fine with that being separate, though we may already have one for that. Can't look at the moment though.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Mar 5, 2018

Member

Woops, I linked the wrong ID when I closed it, but I fixed this in #7417.

Member

loganfsmyth commented Mar 5, 2018

Woops, I linked the wrong ID when I closed it, but I fixed this in #7417.

@loganfsmyth loganfsmyth closed this Mar 5, 2018

@lock lock bot added the outdated label Jun 4, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Jun 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.