babel: JSX runtime appends .js to importSource leading to broken import paths

Bug Report

I would like to work on a fix!

Current behavior

For some reason an extension is appended to the end of the import source string. This is very unexpected as it breaks module resolution in node. It doesn’t play well with the recently established export map feature. One of our Preact users traced the error back to this commit: https://github.com/babel/babel/commit/d6b0822ee9e95321a417b321a6687048faf1c3a1

Here is a full repro case: https://github.com/marvinhagemeister/babel-jsx-runtime-bug

Input Code

export const foo = <div />;

Expected behavior

I expected babel to not add a .js extension to the import path. It breaks node’s export map feature. With export map’s the package.json of the consuming package will look something like this:

{
  "name": "my-package",
  "exports": {
    "jsx-runtime": {
      "require": "./path/to/browser.jsx-runtime.js",
      "import": "./path/to/import.jsx-runtime.mjs"
    },
    "jsx-dev-runtime": {
      "require": "./path/to/browser.jsx-dev-runtime.js",
      "import": "./path/to/import.jsx-dev-runtime.mjs"
    }
  }
}

This worked fine with recent versions of babel before the mentioned commit was added.

Babel Configuration (babel.config.js, .babelrc, package.json#babel, cli command, .eslintrc)

Filename: babel.config.js

// babel.config.js
module.exports = {
  plugins: [
    [
      "@babel/plugin-transform-react-jsx",
      { runtime: "automatic", importSource: "preact" }
    ]
  ]
}

Output:

import { jsx as _jsx } from "preact/jsx-runtime";
export const foo = _jsx("div", {});

Environment

  System:
    OS: Linux 5.8 Arch Linux
  Binaries:
    Node: 14.13.0 - ~/.nvm/versions/node/v14.13.0/bin/node
    Yarn: 1.22.10 - /usr/bin/yarn
    npm: 6.14.8 - ~/.nvm/versions/node/v14.13.0/bin/npm
  npmPackages:
    @babel/cli: ^7.12.1 => 7.12.1 
    @babel/core: ^7.12.3 => 7.12.3 
    @babel/plugin-transform-react-jsx: ^7.12.1 => 7.12.1

Possible Solution

Revert this commit: https://github.com/babel/babel/commit/d6b0822ee9e95321a417b321a6687048faf1c3a1

Additional context

https://github.com/preactjs/preact/issues/2801

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 2
Comments: 20 (16 by maintainers)

Most upvoted comments

Idea: what if instead of doing all that magic that the plugin is doing now we could let users specify the full import specifier: preact -> preact/jsx.-runtime.

That way everybody could set it to whatever they need. This would avoid the weird scenarios we are in right now and would drop a bit of code from babel’s repo. Would be easier to maintain too.

I think in general having babel do module resolution is a slippery slope. There are too many configurations, tools and environments to take care of and with custom user resolvers on top.

What do you think?

marvinhagemeister on Oct 18, 2020

Fixed in @babel/helper-builder-react-jsx-experimental@7.12.4

nicolo-ribaudo on Oct 19, 2020

I am good to revert d6b0822 but we should coordinate with the React team. As for react team, I suggest specifying “exports” and those two runtime:

{
  "exports": {
    "./jsx-runtime": "./jsx-runtime.js",
    "./jsx-dev-runtime": "./jsx-dev-runtime.js",
    "./": "./"
  }
}

So both require("react/jsx-runtime") and require("react/jsx-runtime.js") will work. I lean to discourage the usage of explicit extension as in the future we may have react/jsx-runtime.cjs or react/jsx-runtime.mjs but it is impossible to point ./jsx-runtime.js to ./jsx-runtime.mjs in exports.

JLHwung on Oct 19, 2020

The ideal solution would be that we revert the commit that introduced the regression, and React adds "exports" to package.json to make the extension optional.

If someone really needs to further customize the import path (for example, because using a CDN that doesn’t resolve the exports field), they can use https://www.npmjs.com/package/babel-plugin-module-resolver.

nicolo-ribaudo on Oct 19, 2020

Ok, thank you for the explanation - the gist of it is that exports map are required for ESM in node to support extensionless imports and webpack matches their behavior (only for type: "module" and .mjs).

As mentioned here and in other issues - adding extensions in the emitted code is being problematic for a couple of reasons and I believe that it should be avoided. I understand that it’s unfortunate that things break right now because of it - but node’s semantics are very new. The ESM support in node has been released just this month - so it’s understandable that some packages are not yet ready in full for it.

Indeed. But type: “module”, webpack 5 and jsx-runtime… these are new stuffs. I don’t expect legacy versions will work with them.

I partially agree with that. It was already a great gesture from the React team to ship runtimes for all~ React versions. I suppose they could add exports map in a similar fashion to all of them, just to avoid confusion etc. It makes sense given how many users they have.

As to other packages - jsx-runtime is indeed very new so I don’t really see how old packages would be affected by this at all? They will just use the classic runtime after all which doesn’t even emit any implicit imports at all so you have freedom to write them in any way that you see fit, considering your environment etc.

As to the current webpack’s situation - just don’t use .mjs for now. If you rename your file to .js then webpack will gladly resolve the react/jsx-runtime entrypoint.

Andarist on Nov 27, 2020

Oh and for the original issue (https://github.com/facebook/react/issues/19905), it seems that some CDNs correctly support extension-less imports if they are declared as "exports" in package.json: try running await import("https://cdn.skypack.dev/preact/jsx-runtime") in your browser’s console.

nicolo-ribaudo on Oct 19, 2020