fix: forward options when calling this.resolve

[pcattori]

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]

It still needs skipSelf: true I think?

[pcattori]

From the Vite docs for this.resolve:

The default of skipSelf is true

But no harm (I think?) in setting it explicitly too if you prefer.

[aleclarson]

Oh nice. That's a welcome change from Rollup :)

[IlCallo]

Isn't this a breaking changes for people still using Vite 3-4?
Rollup changed the default into v4, which is used by Vite 5, thus this is a breaking change and would need a proper major release

[aleclarson]

@IlCallo Ah good point! I think I'll explicitly set skipSelf: true for backwards compatibility.

[aleclarson]

Fixed in v4.2.3

[IlCallo]

Thanks 😁

[aleclarson]

Released in v4.2.2

[pcattori]

@aleclarson so swift! 🙏