Implements relativeRuntime

[arcanis]

Q	A
Fixed Issues?	Fixes #10355
Minor: New Feature?	Yes
Tests Added + Pass?	Yes
Documentation PR Link	babel/website#2091
Any Dependency Changes?	No
License	MIT

This PR adds a counterpart to absoluteRuntime named relativeRuntime which, when used, will hardcode the relative path between the transpiled file and the runtime within its import tag. This will help with setups where the runtime comes from a package deep within the dependency tree (think Next.js or CRA) and you want the build artifacts to work the same way across various machines (where absolute paths would break).

[babel-bot]

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

[arcanis]

🥳 Ready to review, @nicolo-ribaudo!

[nicolo-ribaudo]

Could you create a "fake" node_modules/@babel/runtime in packages/babel-plugin-transform-runtime/test, to avoid having the fixtures depend on how yarn hoists the "real" @babel/runtime?

[nicolo-ribaudo]

Suggested change

	"The 'absoluteRuntime' and 'relativeRuntime' settings cannot be enabled at the same time",
	"The 'absoluteRuntime' and 'relativeRuntime' options cannot be enabled at the same time",

(I think? I'm not a native speaker)

[arcanis]

Yep, plus it's more in line with the other messages 👍

[nicolo-ribaudo]

Could you create a "fake" node_modules/@babel/runtime in packages/babel-plugin-transform-runtime/test, to avoid having the fixtures depend on how yarn hoists the "real" @babel/runtime?

[nicolo-ribaudo]

I think that this implementation has a bug when the input file is not in the same folder as the config file.

@babel/runtime should be resolved (here the PR is correct) relative to "config file path + relativeRuntime option" (the dirname parameter received by the plugin), so that this has the same meaning for every file:

{
  "plugins": [
    ["@babel/plugin-transform-runtime", {
      "relativeRuntime": "."
    }]
   ]
}

On the other hand, the path put in the import statement should be relative to the input file: you should use this.dirname (which you can get inside the post(...) method using path.dirname(this.filename)).
If this.filename is undefined, it should throw an error because it isn't possible to locate @babel/runtime relative to the input file.

cc @loganfsmyth Are my expectations the same as yours? (#10355 (comment))

[nicolo-ribaudo]

I think that this is wrong. Even if it is resolved starting from ./subfolder, the location of @babel/runtime relative to this file has only three ../../../.

I would expect a test for the string version of this option to have another node_modules folder inside ./subfolder: then this path should look like

var _classCallCheck = require("./subfolder/node_modules/@babel/runtime/helpers/classCallCheck");

[loganfsmyth]

This doesn't match my expectations. Wouldn't we need to calculate this path relative to the JS file being compiled, rather than using the directory?

I also wanted this to strictly be a boolean option because I think of the flags this way:

• absoluteRuntime
  • false to just inject @babel/runtime/<whatever>
  • true to resolve that to a specific path first
• useRelativeRuntime
  • only allowed when absoluteRuntime is true or a string
  • false - current behavior
  • true - take the resolved path from before and resolve it relative to the JS file it is being inserted into, instead of inserting an absolute path.

If I were doing this from scratch, these might have been nicer as

type Opts = {
  resolveRuntime?: boolean | string,
  resolveRelativePath?: boolean,
};

or

type Opts = {
  resolve?: boolean | string | {
    dirname?: string,
    relative?: boolean,
  },
};

or something

[arcanis]

Yep you're both right - I'll update this PR. Would you know what's the best way to obtain the path of the file being processed during transformation? Local variable set during pre(file), maybe?

[loganfsmyth]

The same way we assign this.addDefaultImport in pre, we should probably assign this.runtimeModuleName generated from moduleName and this.filename.

Note that this.filename can be undefined, so we should throw a useful error in that case.