aleclarson / vite-tsconfig-paths

Support for TypeScript's path mapping in Vite
MIT License
1.23k stars 44 forks source link

fix: forward options when calling `this.resolve` #128

Closed pcattori closed 7 months ago

pcattori commented 7 months ago

Currently, passing only skipSelf and not forwarding options causes custom to be set to undefined, but other plugins rely on the presence of custom for correctness.

For example, Remix would like to throw whenever .server modules are resolved from the client bundle. We'd like to show something like:

Server module {original ID} is being referenced in the client bundle by {importer}

We need the resolved ID to check if the actual file path contains .server, but we need the original ID for the error message. So we call this.resolve to get the resolved ID. To make sure our plugin doesn't participate in further resolution, we rely on custom to setup some custom logic. But since vite-tsconfig-paths is also used in this project and it sets options with custom as undefined implicitly, the data set in custom is lost and our custom logic never triggers.

Additionally, omitting options altogether results in this warning from @rollup/plugin-commonjs:

[plugin:commonjs--resolver] It appears a plugin has implemented a "resolveId" hook that uses "this.resolve" without forwarding the third "options" parameter of "resolveId". This is problematic as it can lead to wrong module resolutions especially for the node-resolve plugin and in certain cases cause early exit errors for the commonjs plugin.
In rare cases, this warning can appear if the same file is both imported and required from the same mixed ES/CommonJS module, in which case it can be ignored.

Since they can't reliably check that you forwarded the options, they just check that options was passed in at all, but its clear from the message that the intent is to forward options.

aleclarson commented 7 months ago

Released in v4.2.2

pcattori commented 7 months ago

@aleclarson so swift! 🙏