Import the correct ./typeof.js helper in @babel/runtime#14081
Import the correct ./typeof.js helper in @babel/runtime#14081nicolo-ribaudo merged 1 commit intobabel:mainfrom
./typeof.js helper in @babel/runtime#14081Conversation
|
You can "test" this (i.e. prevent regressions) by unignoring |
| * @param {*} node The string literal contains import path | ||
| * // returns ast`"./setPrototypeOf"` | ||
| * @example | ||
| * adjustImportPath(ast`"@babel/runtime/helpers/typeof"`) |
There was a problem hiding this comment.
The import "@babel/runtime/helpers/typeof" is generated by @babel/plugin-transform-typeof-symbol which runs before the buildRuntimeRewritePlugin plugin. In this case I will prefer just convert "@babel/runtime/helpers/typeof" to "./typeof": So this plugin does not accidentally support "@babel/runtime/helpers/another-helper".
There was a problem hiding this comment.
If there are injected imports for other helpers, wouldn't we want to transform them too?
There was a problem hiding this comment.
Yes they should.
Not related to this PR: I realized that the generated helper will target to browserlists's default in Babel 8 since we didn't supply a target in the build-dist. I guess it's fine to bump the targets but we should convey that in the changelog.
72f4ad0 to
8244609
Compare
|
Thanks for the feedback, I updated the PR with your suggestions :) |
./typeof.js helper in @babel/runtime
Context
When a helper depends on another helper which is not explicitly imported (see code snippet below), the
babel-plugin-transform-runtimewill inject the helper at the top but it will not inject it in the proper module format version.Let's take the
possibleConstructorReturnas an example. The helper itself does not "explicitly" depend ontypeof:babel/packages/babel-helpers/src/helpers.ts
Lines 618 to 630 in 89cab43
When building
possibleConstructorReturn(build-dist#247) thebabel-plugin-transform-runtimeplugin will run over it and inject the unimportedtypeofat the very top as follows:+ import _typeof from "@babel/runtime/helpers/typeof"; import assertThisInitialized from "./assertThisInitialized.js";The issue is that the injected module format is CJS instead of being picked up by the context. The desired output should be the following:
The solution
To fix the issue I updated the adjustImportPath function to rewrite the imports generated from the
babel-plugin-transform-runtimeas relative path imports.I realize that this PR is just patching the problem rather than making the transform runtime plugin import the proper module format in the first place. Doing that, however, would require us to do some rethinking of the build-dist file, which considering the issue was marked as "good first issue", I deemed it was out of scope. Let me know if I should proceed otherwise.
Testing
Guidance on testing thebuild-distfile is appreciated.Updated: Commited the helper generated code to avoid regressions.