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

[Feature Request] Skip resolving dependencies that are required inside a try/catch if the resolver throws a MODULE_NOT_FOUND error #1127

Open
paul-soporan opened this issue Apr 8, 2021 · 2 comments

Comments

@paul-soporan
Copy link

When a require call is inside a try/catch (e.g.

try {
  require(`dep`);
} catch {
}

), ESBuild's default resolver treats it as external and doesn't try to resolve it. This changes when I add a plugin that registers onResolve, as ESBuild will forward the require call to the resolver. The problem is that the resolver loses the context of the require call and always tries to resolve it (throwing a MODULE_NOT_FOUND error if it can't be resolved), even if the require call is inside a try/catch.

My suggestion would be to check if the error thrown has error.code === 'MODULE_NOT_FOUND' and skip it if the require call is inside a try/catch.

An alternative would be to add a new args.optional property on the resolve args so that resolver plugins can manually skip the dependency if the resolution fails.

Use case: I want to make @yarnpkg/esbuild-plugin-pnp not throw when optional peer dependencies that are required inside a try/catch are not provided by the parent (https://github.com/yarnpkg/berry/pull/2717/checks?check_run_id=2292511030#step:4:177).

@evanw
Copy link
Owner

evanw commented Dec 4, 2022

I'm closing this issue because this is about the @yarnpkg/esbuild-plugin-pnp plugin, but that plugin is no longer relevant as esbuild now implements Yarn PnP natively. Also esbuild already ignores resolution errors for require inside try/catch, so I believe (but haven't double-checked) that if a plugin wants to do this it can just not resolve the path (i.e. return undefined) instead of throwing a specific magic error code and this will already work.

@evanw evanw closed this as not planned Won't fix, can't repro, duplicate, stale Dec 4, 2022
@arcanis
Copy link

arcanis commented Feb 12, 2023

@evanw This isn't a PnP issue - any custom resolver may hit the same issue. Since Esbuild doesn't report whether the resolution is within a try block, plugins can't know whether they need to return undefined or an actual error message.

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

No branches or pull requests

3 participants