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

fix(esbuild-plugin-pnp): respect resolveDir #4569

Merged
merged 1 commit into from
Jun 24, 2022

Conversation

jenseng
Copy link
Contributor

@jenseng jenseng commented Jun 22, 2022

On-load callbacks can provide a custom resolveDir for virtual modules; if set, it should be used for resolving paths in that file. importer is only guaranteed to be a file system path if the namespace is file.

For context, see: https://esbuild.github.io/plugins/#on-resolve-arguments

What's the problem this PR addresses?

If another esbuild plugin creates virtual modules with relative imports, esbuild-plugin-pnp will incorrectly resolve them, since it assumes importer (the virtual module) is a real file path, and there are no such guarantees. This bug was observed after integrating esbuild-plugin-pnp into remix core and attempting to convert virtual modules to use relative imports.

How did you fix it?

Updated esbuild-plugin-pnp to use resolveDir (if set):

  • For modules in the file namespace, this is equivalent to path.dirname(importer), so the behavior there is unchanged
  • For plugins that set a resolveDir, esbuild-plugin-pnp will now pick it up and use it <- this is the fix
  • For plugins that don't set a resolveDir, esbuild-plugin-pnp will still fall back to importer and baseDir

Also added an end-to-end test to verify the behavior (fails on master, passes with the fix).

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

On-load callbacks can provide a custom `resolveDir` for virtual plugins; if
set, it should be used for resolving paths in that file. `importer` is only
guaranteed to be a file system path if the namespace is `file`.

For context, see: https://esbuild.github.io/plugins/#on-resolve-arguments
@jenseng jenseng force-pushed the fix-esbuild-resolve-dir branch from 16e894f to f37a131 Compare June 23, 2022 19:54
@arcanis arcanis merged commit 50394f5 into yarnpkg:master Jun 24, 2022
merceyz pushed a commit that referenced this pull request Jul 20, 2022
On-load callbacks can provide a custom `resolveDir` for virtual plugins; if
set, it should be used for resolving paths in that file. `importer` is only
guaranteed to be a file system path if the namespace is `file`.

For context, see: https://esbuild.github.io/plugins/#on-resolve-arguments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants