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

Astro >= 4.5 Vite changes make it not workable for environmentally configured apps #10537

Closed
1 task
narration-sd opened this issue Mar 22, 2024 · 11 comments
Closed
1 task
Labels
needs triage Issue needs to be triaged

Comments

@narration-sd
Copy link

narration-sd commented Mar 22, 2024

Astro Info

Astro                    v4.1.1  // this is incorrect; see just below
Node                     v20.11.1
System                   Windows (x64)
Package Manager          unknown
Output                   hybrid
Adapter                  @astrojs/netlify
Integrations             @astrojs/react
                         @sanity/astro

Actual Astro:
$ npm ls astro
@narration-sd/sanity-astro-presentation@0.2.0 C:\vger\projects\astro\sanity-astro-presentation
`-- @example/basics@0.3.0 -> .\apps\example
  +-- @astrojs/netlify@5.2.0
  | `-- astro@4.5.8 deduped
  +-- @astrojs/svelte@5.2.0
  | `-- astro@4.5.8 deduped
  +-- @astrojs/tailwind@5.1.0
  | `-- astro@4.5.8 deduped
  +-- @astrojs/vue@4.0.9
  | `-- astro@4.5.8 deduped
  +-- @narration-sd/sanity-astro@0.2.3
  | `-- astro@4.5.8 deduped
  `-- astro@4.5.8

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

any browser; not involved

Describe the Bug

Outline or Precis

What happened here is:

  1. Astro release 4.5 onward broke my months-solid developing application, from a small code change that slipped in
  2. I got some quick advice, to use the old Vite loadEnv for environmentals, rather than their long primary import.meta.env
  3. Using this needed restructuring significant code, but I did it -- and started getting streams of truly odd errors
  4. I worked out that loadEnv causes the errors, upsetting Vite/Rollup's idea of what belongs in client vs. server
  5. Restructuring again to eliminate those situations, the complex app (Sanity Visual Editing and Astro app development framework which 'automatically' lets Astro code become live for editing') took off running fine again, in all local dev and build tests.
  6. However, the bonus turned up when I tested deployment. Netlify broke at the start, with an error message that was a) not possible to employ in their system, and b) pointed to a two-year trail of quite thoughtful fix attempts on the problem, none of which worked in trying here, as they had not for many others.
  7. From the analysis of the underlying problem available in those fix attempts, it's pretty clear that it's Vite and its use of in-node_modules 'additions' to what npm expects, that is the issue -- and this time npm isn't having it. They have coooperated with Vite on other matters, such as virtual modules, exceptioning their rules, but not here.

Ok. Then true details may vary on that last story, but the essence is that Astro has been made unworkable for perfectly normal and needed code involving integrations requiring environmentals where they're called in astro.config, and also hypersensitive to specific code patterns likely to be used, throwing mystery errors.

  • I see this as something Astro wouldn't want itself, and it's a real roadblock for persons I deal with dailiy who build for example applications on the very popular Sanity.io framework, for whom I'm working to create a mutually enhancing situation with Astro, with Astro's great easy on-ramps (to extensive abilities) posture. Futures beckon, not so?

  • Thus I've also proposed a very straightforward way to remove current problems, and very effectively handle what I've been told by the developer was the intention of the problem code addition, which is quite valid and thoughtful in itself.

  • This plan outline is in the initial comment below, on this issue report. Short, and I'd think pretty palatable.

  • Should you want to observe just what happened, assure the details for yourself, those are in the needfully long section that follows, with screenshots -- up to the blocking error at Netlify..

Thanks very surely for understanding :) . And you can skip, from here, to the easily implemented workaround with long-term solution, in that initial comment....

The Progress Story, with the Details

This is a broad claim, that this upgrade is iunworkable, but I feel it's justified, and to get proper attention -- as after making all the necessary discoveries and changes to work with what's been done, Netlify will absolutely refuse to serve the result, even though it has shown to build (and dev) perfectly well.

Evidence to follow, and i must mention that I really regret having to go to such lengths to bring problems out, on a system I have great respect for, and for all those who have been contributing to it. The project these problems show up on is in fact an alpha-stage full integration app development environment, intended to provide Astro projecs with Sanity.io's most advanced visual editing abilities, and that should give a pretty clear idea of motivation, besides long friendliness with some of your top team members.

There is the requested 'compact demonstration' provided this time, not of everything as this isn't really feasible, but sufficient to show the most problematic errors clearly. Prior reports such as #10469 have been closed in favor of this one, which summarizes all learned through troubleshooting.

To describe the central problems, then, there are many aspects where the surely well-intended switch in direction, that through an un-documented, two-days-before-release change in the entire basis of Vite handling for Astro, has caused severe problems. I'm writing this issue to collapse the known points now, after a solid week of troubleshooting to unravel the span of real consequences, and what is likely the basis for several quite challenging points encountered along the way.

Even with this intention, what follows can be only a summary level description, as there were so many interlinked problems for a developer, in situations varying according to what else was operative and being tried. These were not much fun to sort out.

  1. it's no longer possible to load environmental variables which configure Astro Integrations in any safe, normative according to Vite doc way. import.meta.env now fails for this, because we can no longer set envPrefix to fit names required, nor if memory serves from earliest testing. are they loaded now at the proper event time, to be available at the exported integrations config.

  2. we are suggested to revert to the antiquated, hardly documented loadEnv for environmentals. But this has no prefix control, just 'all' or 'VITE_', though it's true it will load environmentals available at config time.

  3. however, it is impossible to use results from loadEnv in Astro client-only components. Attempting to export and import them will result in Vite apparently trying to load all visible Node elements, warning it is handling automatically , marking 'external' each one, until it reaches something it can't handle and crashes. Thus no Astro build is possible. The 'compact demo' will show a very small example of this..

  4. The answer on discovering this is to go back to using proper import.meta.env vars for client components, which can be exported and imported without difficulties, allowing them to collected in a proper unified config file -- a separate one from whatever astro.config may use as properly. Thus I entirely rewrote the complex configuration system for the Astro-Sanity-Visual Editing arrangements, sometimes having to duplicate handling of environmentals, when they were needed in either pattern for differing purposes..

  5. On completing and proving locally all code changes, I 'simply' deployed to Netlify, as has been a backbone of development validation for months. And encountered an unfixable error. I'll put the screen below, but this error has existed for two years, is punted by Netlify, and though there are numerous 'workarounds' to be found, none of them up to two weeks ago will solve this problem.

Publishing to Netlify appears entirely broken, and once again the problem appears to lie in handling Vite's results when using its older loadEnv method, as this has never appeared before -- things it puts in node_modules which do not properly link on Nellify's build, lore says through an interaction with npm, and perhaps they're related to how we already see it mishandles loadEnv export-imports.

  1. There's yet another very problematic aspect. These changes apparently prevent Astro from using an ordinary vite.config.ts file (though I found it's still imported). One can now only put Vite config inside the astro.config.ts. This is what raises the problem, first of all, of not being able to set envPrefix at a event progress point it would apply for integration variables.

    But it also puts into question whether increasingly detailed and important Vite configurations will even be seen and operate -- complex aliases, virtual modules as are showing up as the 'new popular thing', are among these. Also by forcing this rearrangement, Astro config files get more complex, and Vite documentation becomes much less accessible for 'designer' or 'application' roles, we can surely understand.

    All of this seems very much against the 'easy on-ramps' which attract persons to Astro fundamentally, and which I've worked very hard to preserve and extend in atmosphere being prepared for others.

  2. This should be more than enough, and I will suggest in comments a pattern to revert and work respectfully both with the needs missing here, and the concerns I learned which may have driven the un-noted change, that Astro uses a portion of Vite configuration in its own ways.

  3. But first, let's consider that the errors above also mean that Astro has made itself very fragile for developers using it -- even if the problems might not appear for a given application, or at a given time.

    Just one slip of the import/export issue, for example, and a build will fail with fully inscrutable errors, many pages of them, and no clues -- the Node imports failure mentioned.

    This sort of thing can't be a healthy situation for Astro, and of course it's not so for integrating with other frameworks which should be very apparent and friendly compatriots, in building the commercial opportunities which mean a future for everyone's good work, not so?

These are the main points, and I'll attach screenshots beginning with the irreparable Netlify error -- you're welcome to track down all the 'fixes' others have tried, which by trying them were found not to work here, as they haven't in cases for others.

The 'minimal demo' should show direct basis of other problems, and you can also see it below.

I'll add a comment proposing what feels a good way forward.

Screenshot 2024-03-21 204515

Here's the working and the non-working from the minimal demo.

Screenshot 2024-03-22 135534

This is an example of the build failure for attempting to import a loadEnv acquired variable into an Astro client-only component. It's compact only because it's taken on the very simple 'minimal demo' app:

Screenshot 2024-03-22 163500

What's the expected result?

The expected result would be:

  • Astro 4.5 or any other should not mean a substantial rewrite of apps
  • Vite config should work normally as expected, from vite.config.ts
  • Environmental should entirely operate as they always have, from the very flexible and dependable pattern Vite itself entirely promotes as current: import.meta.env.
  • no build-stopping or deployment-stopping errors should appear, for ordinarily structured applications using that centrally powerful feature of Astro, the client-side islands
  • any present and future Vite abilities, as they grow rapidly, should be available wherever Astro can avoid colliding with them

And I would add in due respect: that if there are potential collisions with Astro's internal uses of Vite abilities, then those should of course be protected. A very compatible would be to use straightforward filtering to avoid them, with very explicit errors communicating to developers the situation they've hit in any case where a collision has been attempted..


I'm sure I've left something out here; it's been a long week, but you get the idea. Please see the brief drawing-together in the first comment below, for a path suggested so we can come out of this well for everyone.

Link to Minimal Reproducible Example

https://stackblitz.com/~/github.com/narration-sd/github-lerf2k-suvouc

Participation

I'm suggesting in the plan below, a simple reversion to prior expected behavior, and after that, I know and respect that the Astro team have their own ideas forming on futures.

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Mar 22, 2024
@narration-sd
Copy link
Author

In brief, then, how can we deal with the current situation, and with complete respect for valid concerns going forward, which seem to have motivated the breaking changes?

  1. First, I would think to revert the problem changes which appeared alongside the big release of the Astro database in 4.5.

    This will return operation to what it has been over years, and allow application development for anyone to go forwards.

    The vite.config.ts file will go back to operationa, all import.meta.env configuraations will resume working -- including for crtitical Astro Integrations configuration, and there will be no un-fixable Netlfiy failures to deploy.

  2. There are apparently real and valid concerns that some Vite/Rollup operations involved as we know in Astros Integrations system need to be protected.

    For this, it seems a straightforward filtering can be used -- pick off any potentially colliding elements and block them, at the same time providing a clear explanation error report to the console environment of dev runs or builds. These crossovers should be of course normally documented. Now everyone will be informed at any stage, and Astro operation will be secure.

  3. What about the future?

    It's been mentioned that some conisderation is being given to alternate ways for Astro to handle its own configuration, surely especially concentrating on Integrations.

    What seems very important is to retain Vite and Rollup as the way developers work with Astro, as these are the powerful and well-known tools for brining together all the elements of applications.

    If Astro desires or needs to go to another method of handling its internal configurations, especially for the complexities whiich are potential forIntegrations, surely that should be independent of retaining the Vite/Rollup retained for organizing outside application participations.

All of these, of course, for a sensible future -- if the 'world' changes later, then it will be natural then to change with it, and we all will....

@matthewp
Copy link
Contributor

Can you summarize the problem you are having? I'm having trouble reading this much content. I can tell that you disagree with the decision to stop loading vite.config.ts. We can talk about that.

I think you probably understand by now that loading this file was never documented or intentional. I'm sorry that your app broke, but you were depending on an unsupported accident. If we can understand your use-case maybe we can help you find another/better way.

Let's start by describing your use-case and why you were depending on this behavior. Please try and be as succinct as possible in describing it, only 1 or 2 paragraphs.

@narration-sd
Copy link
Author

narration-sd commented Mar 23, 2024

Hi Matthew, yes, I can do that, and if you like put it on top, as a precis. I wrote a precise description because I wasn't getting persons replying to see the depth of the problem. Both in that it's taken a solid week of more than normal work to dig out just what is going on, and arrive at the punchline: that even if you do, following through with the 'why don't you just use this' advice, you end up with an Astro project that can't deploy to Netlify -- for an unfixable reason.

That in itself is a mouthfull. And, as I'm both in that sort of beyond-programming frame of experience to have taken on some degree of 'adventure' in bringing Astro and Sanity together, I'm as much attentive to how easy we can make the on-ramps for that, and this sort of move on the development abilities for normative designer/appbuilder types, small agencies up, is also a killer.

I can put an I did a, b c, and here I am outline in the top; will that make it easier to digest as I think this Saturday morning?

But the story I think has to be there also, to answer the technical experts such as yourself, who will off the top offer the 'you're just using something we wish people wouldn't' answer, before appreciating exactly where that answer leads: to an impasse where the system built is no longer viable. And before that, making it problematic to employ, and possibly eliminating all sorts of Vite abilities we really need to use, now and in the future.

Thanks indeed for stepping in, and I'd assure you that there's a real problem nest here, and an easy solution out of it I'm trying to get going, for everyone...a link to that is now in the short-form intro as well...

Clive

@narration-sd
Copy link
Author

narration-sd commented Mar 23, 2024

@matthewp besides reply you may appreciate for its reasoning just above, I've put a precis as you asked on top of this report, hopefully answering the need there surely is, for brief understanding.

There's a leader there to a direct answer, for the not-so-direct problems.

Fully appreciated, out of the experience underlying all that's here, and again, that you would write to say what was needed. That's the Astro team I've enjoyed over years, showing why it's so good, once again -- and evergreen 🌲

[ n.b. -- I've added a statement that occurred to me later, for the precis, about staying with the flow of technology, which seems a framing that should be attractive to all of us, and help with the understanding here.... ]

@narration-sd
Copy link
Author

narration-sd commented Mar 24, 2024

@matthewp , also @lilnasy & @bluwy since they had been involved, I finally got the Netlify deploy to run, after a long evening Saturday, trying every single proposed fix, including additional I discovered actually on Astro Discord.

None of these made the slightest difference. I even installed the db, since you said there fixes had accompanied it.

What did work came also from a hint around problems of others. I need the Netlify adapter running hybrid for this system to work, but with the hint, I took it out at a level where the basic site can operate.

Bingo. And in working this way, the site happens to include and operate a good bit of the complex Sanity works used in fiull operation, so I'm confident enough it actually does.

The dramatic error, which Arsh suggested to someone might be bogus, apparently is that. The problem is the Netlfiy adapter, apparently in the environment of Astro 4.5.x, as I'm running the same 5.2.0 version quite successfully with hybrid against Astro 4.4.16. Or at least the big error goes away if I remove it...not at this point prepared to rest on more than that.

So. If we can get a fix for the Netlify adaptor allowing hybrid in 4.5.x, I'm prepared to back off the amplitude here, and would do that by framing yet another issue replacing this one, as I have to this point. The focus would then be only on the Vite and its imports change, which you'll know enough from the precis alone how I feel is quite important to back out and handle as suggested, in a sensible way respecting everyone and their sight.

Thanks again for your attention, and am sure there will be follow-through, don't have to ask it, the Astro way.

Best to each, Clive

@arsh
Copy link

arsh commented Mar 24, 2024

@matthewp , also @arsh & @bluwy since they had been involved, I finally got the Netlify deploy to run, after a long evening Saturday, trying every single proposed fix, including additional I discovered actually on Astro Discord.

None of these made the slightest difference. I even installed the db, since you said there fixes had accompanied it.

What did work came also from a hint around problems of others. I need the Netlify adapter running hybrid for this system to work, but with the hint, I took it out at a level where the basic site can operate.

Bingo. And in working this way, the site happens to include and operate a good bit of the complex Sanity works used in fiull operation, so I'm confident enough it actually does.

The dramatic error, which Arsh suggested to someone might be bogus, apparently is that. The problem is the Netlfiy adapter, apparently in the environment of Astro 4.5.x, as I'm running the same 5.2.0 version quite successfully with hybrid against Astro 4.4.16. Or at least the big error goes away if I remove it...not at this point prepared to rest on more than that.

So. If we can get a fix for the Netlify adaptor allowing hybrid in 4.5.x, I'm prepared to back off the amplitude here, and would do that by framing yet another issue replacing this one, as I have to this point. The focus would then be only on the Vite and its imports change, which you'll know enough from the precis alone how I feel is quite important to back out and handle as suggested, in a sensible way respecting everyone and their sight.

Thanks again for your attention, and am sure there will be follow-through, don't have to ask it, the Astro way.

Best to each, Clive

You are tagging me, @arsh, and I'm not who you think I am. I'm not involved in this project. Please make sure you tag the right person.

@narration-sd
Copy link
Author

narration-sd commented Mar 24, 2024

You are tagging me, @arsh, and I'm not who you think I am. I'm not involved in this project. Please make sure you tag the right person.

You're right, and I apologise, Andres; it should have been @lilnasy Arsh -- changed now. Take care...

@narration-sd
Copy link
Author

narration-sd commented Mar 24, 2024

@matthewp et al, this is probably enough for a Sunday, but I'd woken up with an idea, and tested it.

Instead of linking via npm install, I included the other sanity-astro package in the monorepo, using the '*' version method as often do. This again worked fine locally in all regards, hybrid included, but on Netlify got this very interestingly different Astro w/client-only page access crash there:

`An unhandled error in the function code triggered the following message:

Error - cannot test case insensitive FS, CLIENT_ENTRY does not point to an existing file: /var/task/node_modules/vite/dist/client/client.mjs `

Same flamboyant visuals, but this is not the previous crash, though it may be what was hiding behind its erroneous message.

What I suspect it means is that the Vite virtual module which the now directly accessed package provides -- for the Sanity client mentioned --is not successfully arranged on Netlify, though it's always worked transparently before.

I am going to imagine that this is because of the neutering of Vite's abilities which 4.5 introduced, same basic subect pursued throughout this issue. Would you think otherwise? Those abilities are what otherwise set up the client virtual module -- in vite.config.ts of the package -- when it's always been successful for months.

That virtual module success has been the same whether installed npm package, or in-built within the monorepo as today.

Thanks, and to team,
Clive

@narration-sd
Copy link
Author

narration-sd commented Mar 25, 2024

Just to keep progress here, today I created a test to see if the virtual module was the problem.

Seemingly not, which means the 'client' word in the latest error has other meanings. One possibility is that Vite is still trying to treat browser code as server code, under Astro 4.5.x and the hybrid Netlify adapter now ,worked around already for the earlier case. This is what snooping the vite code referenced in the error also suggests.

Sensibly, I really need to test a current 4.5.9 version of Astro, which only has the line removed which has seemed to cause all trouble, which I've tied down more firmly to its origination here.

Getting one to compile inside my project is a bit problematic, due to the different build systems, though I'll try it a little farther. I could publish such a version compiled in withastro's usual way under my own signature easily enough; just have been reticent as I don't want people finding it on npmjs.com.

p.s. Also tried a Vercel deploy. This fails a little more than equally -- more elaborate logs show similar to the original problems with netlify, suggesting again that Vite is very confused after being reduced in function within Astro.

@narration-sd
Copy link
Author

[importantly modified, and with a good successs, so a re-post]

Well, I published an Astro 4.5.9, suitably masked so I don't think it will be found to interfere in public, and without what has seemed the offending change. Then used it to build on Netlify that way.

it made no change in the immediate situation, but now I may understand why, and one more point that lies beneath the surface to be tried tomorrow may well bring this to full use. But pressing on, I've got other answers.

I went back to work with two types of simpler app. The second one uses all the imported components, and through trying build after build on Netlify, not a few hours of it, I think I've found the present last issue -- and it looks to be the same as those above.

There was in fact still a sneak path for importing information derived from loadEnv. Which every time, seems to drive Vite crazy -- I backed out to earlier versions, etc. no help. But I have a solid app working Netlify deployed now with all the parts,, and tomorrow I'll see if this doesn't fix the large app system, get it back to operation after two weeks.

There's a big question floating in the air, here, then, but it may also be a simple one, and I should get to that tomorrow, given this fix works on the app of most complexity and concern..

@narration-sd
Copy link
Author

narration-sd commented Mar 26, 2024

I'm closing this now, leaving it for all the useful evidence, having reached two forms of repair, and I thing a healthy proposal for how to proceed in Astro's interests now, in #10569

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage Issue needs to be triaged
Projects
None yet
Development

No branches or pull requests

3 participants