Run & review this pull request in StackBlitz Codeflow.

Hmm, you're right. I didn't think that esbuild could break the \ at any position. I think some performance impact is expected, but unsure by how much.

I wonder if it's better to skip strings altogether though and use a plain __VITE_PRELOAD__ global? Tooling shouldn't mangle that and it should always stay as is. We can skip the quotes regex too? 🤔

Hmm, you're right. I didn't think that esbuild could break the \ at any position. I think some performance impact is expected, but unsure by how much.

I wonder if it's better to skip strings altogether though and use a plain __VITE_PRELOAD__ global? Tooling shouldn't mangle that and it should always stay as is. We can skip the quotes regex too? 🤔

Maybe a plain __VITE_PRELOAD__ is better, so that esbuild will not break this string, and we can remove the quotes in preloadMarkerWithQuote

/ecosystem-ci run

Checking the ecosystem for this to see if it works. If it does, I think we can further improve the code since we're no longer dealing with quotes.

📝 Ran ecosystem CI on 0f0a1cf: Open

✅ analogjs, astro, histoire, ladle, laravel, marko, nuxt, previewjs, quasar, qwik, rakkas, remix, sveltekit, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-pages, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress

Seems like this strategy works fine too. I think we can polish up the code a bit to no longer refer it as quotes.

Seems like this strategy works fine too. I think we can polish up the code a bit to no longer refer it as quotes.

replace preloadMarkerWithQuote with preloadMarkerRE

I think the idea behind the quotes was originally to keep the code as a valid module. Maybe we should use import.meta.__VITE_PRELOAD__?

I think the idea behind the quotes was originally to keep the code as a valid module. Maybe we should use import.meta.__VITE_PRELOAD__?

Do you mean that , an undefined variable __VITE_PRELOAD__ may break the code in runtime ?

It may break if there is a plugin that is checking that all variables are properly defined. I don't know if we are respecting this in all our replacements though, just trying to understand why we have been using quotes so far. I like the idea that our plugins are generating a valid module at each step.

Isn't using plain __VITE_PRELOAD__ a valid module too? It's referring to a global __VITE_PRELOAD__ variable. We also have a fallback that replaces any existing __VITE_PRELOAD__, so it should not exist in the final chunk. If it would today, I think it breaks too because preload() does not handle a "__VITE_PRELOAD__" passed in today.

I suppose the only risk here is that __VITE_PRELOAD__ is less unique than "__VITE_PRELOAD__", so it can be accidentally replaced in user code, but I think it's worth the tradeoff and is equally risky as the existing __VITE_ASSET__* constant.

EDIT: Using import.meta. __VITE_PRELOAD__, esbuild can break them up multiple lines too and wouldn't solve the issue.

It may break if there is a plugin that is checking that all variables are properly defined. I don't know if we are respecting this in all our replacements though, just trying to understand why we have been using quotes so far. I like the idea that our plugins are generating a valid module at each step.

I think that, import.meta.__VITE_PRELOAD__ may have some side effect, but I want to try to check it with 'ecosystem-ci'

Isn't using plain __VITE_PRELOAD__ a valid module too? It's referring to a global __VITE_PRELOAD__ variable. We also have a fallback that replaces any existing __VITE_PRELOAD__, so it should not exist in the final chunk. If it would today, I think it breaks too because preload() does not handle a "__VITE_PRELOAD__" passed in today.

I suppose the only risk here is that __VITE_PRELOAD__ is less unique than "__VITE_PRELOAD__", so it can be accidentally replaced in user code, but I think it's worth the tradeoff and is equally risky as the existing __VITE_ASSET__* constant.

EDIT: Using import.meta. __VITE_PRELOAD__, esbuild can break them up multiple lines too and wouldn't solve the issue.

U R correct, esbuild will break import.meta. __VITE_PRELOAD__ too , so , maybe we. don't need to try =_=

I'll queue this for the next minor then to be safe 👍