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

Remove order: 'pre' from resolveId #33

Merged
merged 1 commit into from
Dec 15, 2024
Merged

Conversation

segevfiner
Copy link
Contributor

This causes the plugin to run too early in Vite, preventing any resolve.alias from taking effect or for any other plugin to handle node builtins, whether builtins or builtinsPrefix is set.

For example, when you want to polyfill via alias some Node builtins, when building for both browser and Node.js.

enforce: 'pre' is still needed to run early enough in the Vite plugins order. Which makes resolveId already run in the right place in the plugin order.

For plain rollup, just place the plugin before other plugins that depend on aliases, for reference, the standard alias plugin doesn't include order: 'pre' as well, to allow control such control.

This causes the plugin to run too early in Vite, preventing any `resolve.alias` from taking effect or for any other plugin to handle node builtins, whether `builtins` or `builtinsPrefix` is set.

For example, when you want to polyfill via alias some Node builtins, when building for both browser and Node.js.

`enforce: 'pre'` is still needed to run early enough in the Vite plugins order. Which makes `resolveId` already run in the right place in the plugin order.

For plain rollup, just place the plugin before other plugins that depend on aliases, for reference, the standard alias plugin doesn't include `order: 'pre'` as well, to allow control such control.
@Septh
Copy link
Owner

Septh commented Dec 7, 2024

Hi,

Sorry for the delay.

TBH, I've always been wondering if having both enforce: pre and order: pre set wasn't a little too much but never took the time to check :)

I've made some testing and I think you are right, enforce should be enough for the plugin to work as expected with both Rollup and Vite.

However, this is potentially a breaking change as it may force some users to reorganize the plugins order in their config, so for now I'll keep this in mind until the next major (which should come out very shortly).

Thanks for contributing!

@Septh Septh merged commit 9ba4732 into Septh:main Dec 15, 2024
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