fix: html env replacement plugin position

[patak-dev]

Description

Fixes #12202 (comment)

normalHooks are post hooks. We need to apply the env replacement plugin as the first preHooks

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[bluwy]

The idea is that user pre-hook plugins can run before htmlEnvHook like in dev:

vite/packages/vite/src/node/server/middlewares/indexHtml.ts

Lines 53 to 59 in 42e0d6a

	preImportMapHook(server.config),
	...preHooks,
	htmlEnvHook(server.config),
	devHtmlHook,
	...normalHooks,
	...postHooks,
	postImportMapHook(),

Maybe we should push htmlEnvHook to preHooks instead of unshift?

[patak-dev]

You would like this so users are able to inject new env placeholders in the HTML to be replaced?

It makes the feature more powerful for users but I'm afraid of the burden we would impose on plugin authors. preHooks added by plugins would need to take into account that in a URL attr there could now be a %VITE_URL% placeholder. And that means they can't process all URLs. That is why I added the first plugin that I felt was safer and easier to explain.

I'm not sure what is best here, the position of the plugin changes quite a bit the way the feature works.

[bluwy]

You would like this so users are able to inject new env placeholders in the HTML to be replaced?

Yeah, or if they want to "see" the raw HTML before Vite replaces it.

It makes the feature more powerful for users but I'm afraid of the burden we would impose on plugin authors. preHooks added by plugins would need to take into account that in a URL attr there could now be a %VITE_URL% placeholder. And that means they can't process all URLs. That is why I added the first plugin that I felt was safer and easier to explain.

transformIndexHtml ordering is based on it's order/enforce option, not the plugin's enforce option though, so a plugin author would only access this if they have transformIndexHtml.order: 'pre', which means they explicitly opt-in to this pre-behaviour.

I think it's good that we carve out an API for "before env replacement" since I think there's a good chance some plugins want to tap into this phase.

[patak-dev]

I agree it is good to have the option to slide before the env replacement. But we would also lose the option to go after it. For example to process a URL in an attr that was replaced. I think both have cons and pros, I'm fine with either.

I was afraid we were adding this in a minor, so we could break plugins. But this isn't a big issue as no one is actually using this yet so some plugins may break when users start to use the feature but it isn't really a breaking change per-se.

[bluwy]

I guess you do have a point too which I've not thought before, and there would always be a tradeoff unless we add a new "almost-normal" hook 😛 In the case of plugins processing URLs, they could still workaround by manually replacing %...% themselves (read from resolvedConfig.env), so it might not be a big blocker. Happy to hear what others think too though.

[antfu]

I think enforce: 'pre' should be able to get the "raw content" just like how transform hook works, that we don't resolve the path etc. If users want to have the HTML after Vite, they could use the normal or post hook.

[antfu]

When using pre, one can't assume the input is a valid format (like pre-transform it can get raw CSS, raw Vue, or anything). This would allow plugins to provide preprocessor or actually inject %...% syntax for Vite to interop etc.

[patak-dev]

@antfu in this case, we would resolve the paths in both options after the envPlugin. So the question is not if it should go before or after Vite processing, but if it should go before or after user hooks with enforce: 'pre'

Option a)

• env replacement
• user pre hooks
• vite processing (resolving paths, etc)
• user normal and post hooks

Pros: pre hooks will see the replaced values and can transform them before Vite processing
Cons: pre hooks can't inject %VITE_VAR% into the HTML (but they shouldn't need, they have access to the env themselves to inject them)

Option b)

• user pre hooks
• env replacement
• vite processing
• user normal and post hooks

Pros: see the raw HTML including %VITE_VAR% placeholders
Cons: plugin authors need to care that pre hooks could see an href attr with %VITE_VAR%, so they need to guard against it if they are processing URLs (the test case added in this PR)

The more I think about it, I'm leaning towards a) 🤔

[patak-dev]

Ah, I didn't see your second message before sending mine. Good to see you aren't worried about having the guards from a plugin author's perspective. Points for b) then.

[patak-dev]

Modified the PR to use option b) so we can move forward with the release. If you both think this is better, let's go with it 👍🏼