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

import() argument shouldn't be evaluated in the arrow scope #84

Closed
nicolo-ribaudo opened this issue Apr 18, 2020 · 6 comments · Fixed by #85
Closed

import() argument shouldn't be evaluated in the arrow scope #84

nicolo-ribaudo opened this issue Apr 18, 2020 · 6 comments · Fixed by #85

Comments

@nicolo-ribaudo
Copy link
Contributor

Reproduction

var babelPluginDynamicImportNode = require("babel-plugin-dynamic-import-node");
var babel = require("@babel/core");

var input = `
let i = 0;
import("./file-" + i++ + ".js");
assert(i === 1);

async function fn() {
  await import(await getSource());
}
`

var out = babel.transformSync(input, {
  configFile: false,
  plugins: [babelPluginDynamicImportNode]
});

console.log(out.code);

Output

let i = 0;
Promise.resolve().then(() => _interopRequireWildcard(require(`${"./file-" + i++ + ".js"}`)));
assert(i === 1);

async function fn() {
  await Promise.resolve().then(() => _interopRequireWildcard(require(`${await getSource()}`)));
}

As you can see in the output code, the first assertion fails because i++ is evaluated asynchronously, and the second one is a syntax error because await is inside a non-arrow function.

Proposed output

We can pass the import argument in Promise.resolve, so that it is in the correct scope:

let i = 0;
Promise.resolve(`${"./file-" + i++ + ".js"}`).then(_ => _interopRequireWildcard(require(_)));
assert(i === 1);

async function fn() {
  await Promise.resolve(`${await getSource()}`).then(_ => _interopRequireWildcard(require(_)));
}
@ljharb
Copy link
Collaborator

ljharb commented Apr 19, 2020

That's a great catch and seems like a very trivial fix. Want to submit a PR?

@nevercast
Copy link

This seems to break Webpack's dynamic imports and code splitting. Since Webpack no longer sees a prefixed string in require() i.e. require('./pages/' + page) but instead sees only the expression _

Has anyone else had issues with "Critical dependency: the request of a dependency is an expression" ?

This SO question covers my problem exactly, but the solution was not sufficient: https://stackoverflow.com/questions/61719327/how-to-use-an-expression-in-an-import-with-babel-webpack

@nicolo-ribaudo
Copy link
Contributor Author

The solution is "don't use this plugin with webpack": webpack supports dynamic imports (since version 3 I think), so you don't need to transpile them.

Also this plugin wasn't meant to be used with webpack in the first place (hence -node in the name): there was https://github.com/airbnb/babel-plugin-dynamic-import-webpack which is the equivalent one but for Webpack.

@nevercast
Copy link

I didn't explicitly bring this plugin into my build - something else must have. I will go hunting there. Thanks for your reply!

@nicolo-ribaudo
Copy link
Contributor Author

nicolo-ribaudo commented Nov 24, 2020

If you are using Babel 7.12, you could try with

BABEL_SHOW_CONFIG_FOR=./path/to/a/file/containing/dynamic/import.js your-build-command

(example: BABEL_SHOW_CONFIG_FOR=./src/index.js npm run build)

it will show the resolved config, and you can share the log output if you need help understanding it.

@nevercast
Copy link

BABEL_SHOW_CONFIG_FOR seems to crash Awesome Typescript Loader, but that's okay because the config is still emitted.

Downside is that there is no plugins listed here at all, seems I've made a mistake. Must be something built into babel/core transpiling my requires exactly the same way as this commit does.

{
  "inputSourceMap": {
    "version": 3,
    "file": "src/js/site.ts",
    "sourceRoot": "/home/josh/projectDir/",
    "sources": [
      "src/js/site.ts"
    ],
    "names": [],
    "mappings": "/* removed */",
    "sourcesContent": [
      /* removed */
    ]
  },
  "sourceRoot": "/home/josh/projectDir",
  "filename": "/home/josh/projectDir/src/js/site.ts",
  "sourceMap": true,
  "babelrc": false,
  "presets": [
    [
      "@babel/preset-env",
      {
        "targets": "cover 99.5% in NZ, IE 11, iOS >= 9, Safari >= 9, not IE <= 10",
        "modules": "auto",
        "debug": false,
        "useBuiltIns": "usage",
        "corejs": 3
      }
    ]
  ]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants