fix: normalize moduleName of absolute runtime

[JLHwung]

Q	A
Fixed Issues?	Fixes #10209
Patch: Bug Fix?	Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Nope 😢
Documentation PR Link
Any Dependency Changes?
License	MIT

The moduleName of resolved absolute runtime should be normalized, otherwise it will return
PATH\\TO\\MODULE on Windows 7, which is apparently incorrect.

I don't know how to add a unit test given that CI runs on Linux only.

[babel-bot]

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

[nicolo-ribaudo]

Would path.posix.dirname work instead?

I don't know how to add a unit test given that CI runs on Linux only.

It's ok; in the future we should use Travis CI to test Babel on windows maybe.

cc @sag1v
Could you try cloning this branch and checking if it works? Neither me nor @JLHwung have Windows to test it.

[JLHwung]

@sag1v Could you test on your machine to see if the test is now fixed on this branch?

git fetch origin refs/pull/10242/head
git checkout FETCH_HEAD
// build babel
make build
// now run the test
yarn jest transform-runtime

[sag1v]

@JLHwung When i run git fetch origin refs/pull/10242/head
I get

fatal: Couldn't find remote ref refs/pull/10242/head

I think its because i'm on a fork and not the actual repo?

Edit
Aw, maybe i could just clone @JLHwung's fork and test it...

[JLHwung]

I think its because i'm on a fork and not the actual repo?

Oh I just assume everyone is cloning babel/babel, in this case you can add it as a new remote

git remote add upstream https://github.com/babel/babel
git fetch upstream refs/pull/10242/head

maybe i could just clone @JLHwung's fork and test it...

Surely you can. Note that git remote add would save your time on make bootstrap and yarn.

[sag1v]

Thanks @JLHwung !

As for the tests, still failing.
When i run yarn jest transform-runtime i get 2 failing tests:

- var _classCallCheck = require("<CWD>/packages/babel-plugin-transform-runtime/node_modules/@babel/runtime/helpers/classCallCheck");
+ var _classCallCheck = require("./helpers/classCallCheck");

- var _classCallCheck = require("<CWD>/packages/babel-plugin-transform-runtime/node_modules/@babel/runtime/helpers/classCallCheck");
+ var _classCallCheck = require("./helpers/classCallCheck");

And when i run make test i get 13 faild tests:
Example:

- "fixtures/sourcemaps/call-identifiers/input.js",
+ "fixtures\\sourcemaps\\call-identifiers\\input.js",

[JLHwung]

When i run yarn jest transform-runtime i get 2 failing tests:

Interesting. It seems like I have replaced a bug with another one. I will investigate later.

- "fixtures/sourcemaps/call-identifiers/input.js", + "fixtures\\sourcemaps\\call-identifiers\\input.js",

This one is related to how sourcemap is generated.

@sag1v Thank you for posting test results. I guess I had better get a Windows environment running and go through these issues.

[JLHwung]

@nicolo-ribaudo My perspective on this PR has changed: Even if we may normalize the path separator in the import call, the transformed result injected with absolute path can only run on Windows -- we cannot get rid of the leading disk label anyway.

As Windows build of Node.js support both \ and / in module resolution. The transformed result is effectively working on Windows machine. Therefore I think we don't need fix this.

The test failure is expected, as the behavior of injecting absolute path runtime, is os-dependent.
If we agree, we should close this PR and mark it as won't fix.

@sag1v There are other issues, i.e. source maps that may need fix. The discussions above focus only on the absoluteRuntime of transform-runtime. We may track these issues on different thread.

[nicolo-ribaudo]

I'm ok with closing it as "wontfix", but I'd really like to have an option to skip running a test on some platform. Something like this:

{
  "plugins": [ /* ... */ ],
  "os": ["Linux"]
}

Similar to minNodeVersion:

babel/packages/babel-helper-fixtures/src/index.js

Line 167 in 07eaa3c

if (taskOpts.minNodeVersion) {

[JLHwung]

@nicolo-ribaudo Good idea. Let's discuss the os field design on #10252 . Closing this PR now as wontfix.