Astro/Vite interaction appears to create fatal errors when using loadEnv to recover from Astro 4.5 changes.

Astro Info

Astro                    v4.1.1
Node                     v20.11.1
System                   Windows (x64)
Package Manager          unknown
Output                   hybrid
Adapter                  @astrojs/netlify
Integrations             @astrojs/react
                         @sanity/astro

If this issue only occurs in one browser, which browser is a problem?

any

Describe the Bug

This is a follow-on to #10406, where there's a blocking problem apparent from the suggested workaround.

As proposed for the Astro 4.5 truncation of environmental variable features, I've used loadEnv via process.env = {...process.env, ...loadEnv(mode, process.cwd()), ''} to collect all in a single place, then plumbed them to where they''re used..

In Astro 4.4.15 and 4.5.5, here's what goes right - and then very wrong. Without this fault, I've proved the complex app all works fine - Sanity's Presentation live editing in Astro, in a way that preserves the Astro code -- but this is by substituting a hard value for the problem environmental, as cannot be done in the actual application:

I import many environmentals, but all but one of them are used in Sanity (React) libraries.
all the variable values arrive; I can log them, can see they work where required by Sanity
I import one environmentally derived value only, into a module of my own
this module is used by each of my React display components, which themselves end up in Astro islands on a page
jumping ahead, there are thus many identical errors on one page, and failure of all the components.
each error looks like this.:

`[astro-island] Error hydrating /src/previewable/react/SAPreviewBase.tsx SyntaxError: redeclaration of import viteinjectQuery william-gibson:formatted:327:24 start william-gibson:327 childrenConnectedCallback william-gibson:286 connectedCallback william-gibson:261 (Async: LifecycleConnectedCallback)
william-gibson:171 william-gibson:345`
the mentioned SAPreviewBase is the module that imports the one value, and holds the primary React components

These browser console errors appear as soon as I import this env variable-derived value, which is simply exported on a constant, or I tried returning it in a function also, which didn't help .

As soon as I import from the variables collection module, the errors appear -- no need to actually use the import, as I proved by disconnecting it.

I substitute a static value in the using file, comment out the import, and boom -- everything works, no errors

This problematic (simple string) value imported from the collector file's env access is actually a URL, and I seem to find mentioned that the vite internal function implied in the error has something to do with urls.

I discovered @vite-ignore, which perhaps may be supposed to help with this kind of error on urls, sprinkled it around any evident places, but no help arrived.

The app, and the system becoming well developed around it which links these two fine frameworks, absolutely depends on being able to get that URL from an environmental. Potentially, of course, others in future.

What might you suggest?

Thanks, Clive

What's the expected result?

Proper operation in Astro islands would be to be able to use all loadEnv-provided environmental values without upset -- not exciting the Vite-involved 'injection', and/or not failing if it must happen.

Link to Minimal Reproducible Example

not readily created -- undertaken if needed

Participation

[ ] I am willing to submit a pull request for this issue.

Thanks for creating a separate issue!

A reproduction would be helpful. You can start with this one showing the use of loadEnv.

Hi Arsh, and thanks sincerely for your pleasantness.

I've gotten farther on problems here, still all related to the original issue, and it isn't, I'm afraid, pretty, while the hours add up running very odd errors to ground.

Carefully considered solution proposed at the end, so let's see what generates that thought.

a fresh problem showed up, unrelated to Astro, but brought in by npm installs updating another package whose very recent versions upset Vite badly also. But that's out of the way.
I kept getting very strange results on this complex project, not able to run it after months of everything just fine, so I did an npm create astro, and tried loadEnv there. It works as promised...however, you can hear the 'but...', can't you.
I could not get loadEnv to operate in the real project, since as soon as I added loadEnv to the file where the config resides, Vite started generating very bad, uncorrectable errors. We'll get to that...
So I went ahead methodically, and in spite of the mess it makes, I moved all the environmental and related structuring into astro.config.ts
so far so good, but of course none of the environmentals were being picked up. So I added the loadEnv call to get them available, this time into astro.config.ts itself. I have a console.log showing they are all indeed present at the very first,.
And then Vite has gone wild again, just the same, and the build (or equivalent dev) fails.

Here I probably need to explain a few things.

a substantial amount of the config I need is for Sanity, and it is passed through an Astro integration built by Sanity -- not the one you may have seen in earlier months, but simiilar. So you can see there's an order of execution issue at least waiting to happen.
which is why I need the config available before the Astro exported config which includes the integration can run. But the log proves that they are -- just as they had been when it was possible to use import.meta.env to get them.
Why can't I use import.meta.env with Astro 4.5? I think this is much simpler than I originally guessed, that it's only because Bjorn @bluwy 's unannounced mod took away the ability to set Vite's envPrefix -- in the execution order where it's needed, thus hiding the PUBLICSANITY way I need to have my environmentals named to satisfy other requirements
thus we bring in loadEnv, which has created several varieties of big upset as I've worked through variations attempting to get this simple workaround to operate.. So now it's time to show you the lastest ones.

The Big Upset

again. loadEnv is functioning and delivering my variables. If I comment it out, the build runs to a late stage and fails on the first of them being missing, as expected.
but expected is not what happens when loadEnv is in use. First, the build progress a little, and then begins emitting pages of lines like this:

02:15:45 [WARN] [vite] [plugin:vite:resolve] Module "node:fs" has been externalized for browser compatibility, imported by "C:/vger/projects/astro/sanity-astro-presentation/node_modules/vite/dist/node/index.js". See https://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.

after very many of these rather obvious upset over ordinary modules which had been fine in the environment, Vite finally fetches up on one that for some reason it can't externalize itself, of all things fsevents. So I put in a Rollup external entry for fsevents....and try again.

The result is not happy -- and if you need to know, 'sanity-astro-presentation' is my own module package, full of 'interesting' things from Sanity, React code and exported components, and other gems of the firmament like Nanostores:

02:15:57 [vite] ✓ 3854 modules transformed. 02:15:57 [ERROR] [vite] x Build failed in 13.36s "basename" is not exported by "__vite-browser-external", imported by "../../node_modules/rollup/dist/es/shared/parseAst.js". file: C:/vger/projects/astro/sanity-astro-presentation/node_modules/rollup/dist/es/shared/parseAst.js:11:18 9: */ 10: import { parse, parseAsync } from '../../native.js'; 11: import { resolve, basename, extname, dirname } from 'node:path'; ^ 12: 13: const ArrowFunctionExpression = 'ArrowFunctionExpression'; Stack trace: at getRollupError (file:///C:/vger/projects/astro/sanity-astro-presentation/node_modules/rollup/dist/es/shared/parseAst.js:375:41) at Module.error (file:///C:/vger/projects/astro/sanity-astro-presentation/node_modules/rollup/dist/es/shared/node-entry.js:13651:16) at ModuleScope.findVariable (file:///C:/vger/projects/astro/sanity-astro-presentation/node_modules/rollup/dist/es/shared/node-entry.js:11802:39) at FunctionBodyScope.findVariable (file:///C:/vger/projects/astro/sanity-astro-presentation/node_modules/rollup/dist/es/shared/node-entry.js:6016:38) at CallExpression.bind (file:///C:/vger/projects/astro/sanity-astro-presentation/node_modules/rollup/dist/es/shared/node-entry.js:4732:23) npm ERR! Lifecycle script build failed with error: npm ERR! Error: command failed npm ERR! in workspace: @example/basics@0.3.0 npm ERR! at location: C:\vger\projects\astro\sanity-astro-presentation\apps\example

Position and Proposal

Arsh, Bjorn, this is a situation pretty much out of control, isn't it?

It's too late to try it tonight, but just possibly the situation is now localized enough that it might be possible to box it up as a minimal(??) demo. If without the armature that's real , this problem will show itself by what you see, which is essentially breaking everything in sight...

One factor in this that's unusual is that the Astro integration for Sanity has a bit of Vite code to do a critical virtual module or two in it. That's the only thing I can imagine as a possible cause, though the rest of code imported throughout my app from Sanity is very substantial.

But with all the effort this situation has so far cost, is it really too much to ask of the generally extremely able and so often generous-minded Astro team, to simply either back out this un-announced mod --- at least to the extent where I could get envPrefix set in a local vite.config.ts, or some other equivalent place if you are afraid that would be taken advantage of, so that it will be ready in the right execution order that import.meta.env will come back to functioning before the exported config in astro.config is run?

Thanks, Clive

Ok, @bluwy and @lilnasy , Bluwy and Arsh, I've found the site of the problem, though its nature I can scarcely believe myself, except that I have the full interestingly featured Sanity Astro content alpha up and running now on Astro 4.5.6, as nicely as ever.

It's one line change, just the import of one of those environmental items from the config source. There are only two possibilities I can think of:

that there's some kind of hidden circular reference that Vite hasn't picked off as usual, but gets entirely confused by, or,
it's something to do with the invisible 'injection' Vite apparently does when it thinks it's got a web path, and that is what is entirely confusing it(self).

I remove the import, put a simple string in its place, and reality returns, to all previous months of construction.

I have more to do, but it's been long days chasing this, so not more today.

I do think this mishap underlines the arguments for not having made this undocumented mod. And yes, there's some docs language about what you can use in astro.config, but it is not in any way clear about just what is or isnot permitted. The driver is really order of operations, and that needs to be explicated before anyone could realize meanings. What's there is only a non-informative broad brush, and in a way easily missed anyway.

I almost, but not quite, smile, that I can probably replace this problem point with an actual import.meta.env safely. Again, order of invisible operations of Astro will determine this.

I'll close this issue when it seems complete to close it, giving as much information as I can for the next victim. I'm considering whether to remove the horrendous traces; but those are both a warning to you, and have the search points that may show up for others, later.

Please, consider what was said about just permitting the envPrefix to be set, and import.meta.envs to function even 'in the config' as they have for a very long time. This is where we need them to function, when Astro integrations need values. I htink it's clear from the problems just worked through that turning on Vite's loadEnv in parallel with their automated import meta.envs is not necessarily a safe thing to do.

You can shut off everything else you want to, including separate vite.config.ts files sensibly, provided that you give that setting ability otherwise, in the proper flow of events. Do surely, visibly warn about this, thank you, if the file will not be read, as seems missing at this point.

Oh, and how I found this, was by building up the entire architecture piece by piece from a static Astro app. It's a mirror of what you asked for, but far more complicated that you would want to do to get to this problem, though I fully appreciate that the means was asked for. It's just that this time, one had truly to go deep, and the waters I'd promise you, are.

The reason to lobby for leaving what was well enough alone, or keeping the essential that you and Vite publicize as the normative and proper handling of environmentals mostly, is that it is simple.

Astro itself has the great and attractive virtue of being simple on its surfaces, so that it is attractive. Attractive is how you get your numbers, and your success. This is true over on the Sanity side just as well, and is why I'm building as means of promoting the connection with Astro -- for those very many who don't feel themselves technical aces , but as persons who build applications, build support for their families and others.. They are who will really like this....as long as it truly has not hidden traps, is...simple. Not so? Thank you for thinking about it.

Regards to each of you, Clive

a late p.s. -- as already mentioned to Bluwy, I wanted to apologize if have seemed aggressive at any point - just keyed up on this problem solving, going on for many hours of days.

Appreciated some exchange with him about the deeper roots and desirables on the basis change, and hope that helps get a good situation for everyone, including that problems like this one will not be excited.

As mentioned there, I think it's the worst I've seen in a long career, of course exacerbated by thinking I'd somehow made a small error in the complexities of the workaround for the change, rather than a real and very unexpectable fault of a build tool...

Take care, both, and let's see what comes out of this, with a little more time for reflection :)

Ok, I think I have this under control, have worked around it 99%, and then found a proof it isn't really fixable overall, with the Astro 4.5-included changes. I do also have what I feel is a decent way to resolve this, which should respect and satisfy everyone's interests.

The Situation

the basis problem is that Vite becomes confused when its loadEnv is used to gain environmentals, but then even just one value is imported into an Astro area that will be running on the client -- even if it's just a utility file serving an island on the Astro page.
what Vite does then is try to build load for the client-side as if it included Node calls. These are the copious 'making exxternal for you' warnings, pages of them, and then a fail on one it couldn't handle this way, or possibly just ran out of stack or a runaway constraint on it.
the only possible line of solution with the new Astro regime is to entirely separate environmenals handling,, running loadEnv for purely server-side values, andimport.meta.env for purely client-side needs. And never import from one into the other, even if this means duplicating code to avoid it.
you can imagine how awkward this becomes in code, and it has also made the Sanity-Astro application system entirely fragile -- just one false move, and the entrapment in failure will happen, to the person using it. None of this can be acceptable; would any of you accept it? There's a definite commercial interest, for both Astro and Sanity,.
However, the 'feather in the cap' of this problem comes when you do a considerable workaround, as I've built, and see it run in dev. But then as you attempt to deploy, you then run into t two-year-old mystery npm/Netlify error which absolutely prevents this.

None of the available solutions work, in this case. Deployment has been made impossible, for what should be an undemanding app system to enjoy, where the fine systems of Sanity and Astro can offer their abilities together, in ease and in value.

I've given more detailed information on all of this to Bjorn @bluwy , using another channel. The Netlify failure page is eloquent, and emphatic.

The Sensible Solution

first, if this can't be decided, or handled, right away, please just revert the undocumented, two-days-before-4.5 change. Return Astro to how it was. Then we who use it can go forwards. and will thank you.
we want to respect what I've understood to be a sensible desire to protect Astro's special uses of Vite config.
but at the same time, we should not upset all the other needful uses of Vite/Rollup config, nor especially, interfere with the import.meta.env method of using environmental variables, including and especially their use early in the app startup, where they are required by Astro integrations such as Sanity to have their values.
those Vite/Rollup configs need to continue to be able to exercise detail features, such as complex aliases, virtual moduiles, etc., all of which are in current popularity and need of use....

...along with the crucial ability to set value/s for Vite envPrefix so that namings required by other systems can be used.

Thus the solution proposed is:

once again, immediately revert the simple but highly hazardous change just made in Astro, which breaks all these things, without actually effective work-around.
make the partition for Astro internal use of Vite config clear to yourselves, Leave all other Vite config where it belongs and is documented, in the vite.config.ts . Try to minimize any Astro-centric vite: config.
the key to the protections desired: simply filter the incoming Vite config from vite.config.ts, so that it doesn't overlap where you need it not to. Warn clearly from this filter step, when this is attempted, that's all, and don't accept it.
clear documentation for this, of course, as that can occur. But first, fix the situation, and include active informational warnings, would seem sensible, not so?

I think we all understand the motivation, now that the far-reaching and inflexible nature of this problem is clarified.

Thanks for listening, Clive

Closing this in favor of #10537, a compact summary of results now that the investigation is complete, leaving it available for reference.

withastro / astro