-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 creates severe problems and side effects for environmentals, potentially other configs - with resolution #10569
Comments
p.s. one thing I could add, useful if it isn't already clear. A reason this is so challenging to troubleshoot, beyond the insensible messages, is that most of the problems are observable only on deployment. The system must be built, in order to have the improper separation attempt between server and browser client code to occur and fail as it does. And though the hybrid-using system can be built on a laptop, the result can't be previewed there, due to the adapter. Once again, am imagining this is something a customer would be happy not to run into... |
This issue is, again, far too verbose. You don't need to explain things like you're writing a blog post. Just state the problem as concisely as possible. From my reading it is:
Please answer these two questions in as few words as possible. 1 or 2 sentences each should be enough. |
Hi Matthew, and I appreciate you're in the midst of triage with new release, surely.
|
Ok, at least I was consistent; it was late, and it's early now.
|
@matthewp Matthew, reviewing, I realized the error screens above don't quite illustrate the central failure, so I've re-created the best view you can get of it, here.
Remedies are as described in the original post. |
Again I ask that you be less verbose. I cannot help if I need to read so much to understand the problem. Ok, so from what I can tell the problems are now:
For (2), do not load your Astro config in a client component. That is not supported and is not a good thing to do, you are loading a lot of code in the client by doing so. Instead create a file like perhaps I think the crux of the issue is you want a single file where you read environment variables and you want that to be the config file. Let me state very clearly you cannot do that in Astro. You need to use both Please try this and let me know if (1) is still happening. |
Necessarily further:
I'm apologetic, Matthew, if being polite and complete make things difficult, truly. I can understand you, however, I think. |
If the issue here is that
Astro does not solve loading environment variables from different sources into your Astro config. This is not a feature of the framework, it's something you need to figure out depending on your needs, using ecosystem packages or built-in Node.js features. |
Matthew, aren't you narrowing this so your 'works as re-designed' arguments fit? I've handled everything you talk about, and certainly am aware of other methods -- once the ferocious server crashes could be understood. [also memory tempts me to think l The point I see is that you guys are very unnecessarily breaking things in a way that will cause this for others. It may be uncomfortable to have that pointed out, for which I've been respectful and sorry. Would you yourself accept such crashes, without fully understanding them and seeing remedies that were not full of risk? 'Just' using two separate systems, where one can cause such upsets by any inadvertent presence, not even use, of the 'wrong' one., seems to me full of risk. I've addressed whatever prompted the change actually in the suggestions. In a long career as a 'real' consultant, those are the kinds of solutions that have been praised, to 'really work'. |
I guess I could also remind, that it is not imagined 'others' that I see this problem affecting. Patient as I am in it, Astro persons' response so far have caused me to seriously consider -- against -- putting forward the system for Sanity I have months of successful effort into, where I know very well how tense and brittle much of the user base is about anything past what they can glean from copying scaffolds without challenges. And especially, mystery errors. Perhaps I can hide enough and comment enough what needs to go on to avoid their confrontation if you don't open up and fix this, but when they try to extend, as you know such persons also are prone to, for 'features', what then? I'm not and never have been a 'business person', nor other things you may imagine, but I am pretty experienced in what they feel to need, and do appreciate in broadly creative responses. Sanity and Astro could help the future of every one involved, it has seemed. If it can be made approachable and solid. I suspect you'd be interested in some of the approaches I've invented to manage that for them. If we can get there, Matthew...it's been tough enough already, sometimes kind of satisfying.... Out for some exercise, fresh air now, being quite human, and much more of years than you probably think, also :) Clive |
Sorry if this happened to you. It happened to other users, and we always try to fix regressions as fast possible, and we always try to suggest workarounds in the meantime. It's unfair to judge our work like this. There's also the fact sometimes users are unintentionally leveraging issues, and once we fix the issue, things don't work. It happens, it happens and it will happen. Please keep this in mind.
No, although if we can't understand, that doesn't mean we don't accept it. We can't understand your issue. We're asking you to:
We will triage it once we have clear steps. The more succinct you are, the faster your issue gets solved. |
@ematipico, thanks, I appreciate what you say, especially your approach also, and caught it before out the door. I'll think what I can do. With a problem so obscurant of itself, it's hard indeed to reduce what it takes to understand 'steps' to play with it -- I remind that much of it only occurs on deploys. Have you a system where you can actually see inside what happens there? Things like But to see Vite fail by trying to load client code as if it were server code, which is the primary actual failing, this I might be able to cook up a suitable way to demo. I don't know that it can run on StackBlitz, but if so, I could publish a repo. Thanks for now, and on your timezone especially :) |
@ematipico I understand the steps now. It's the same thing as in previous issues. @narration-sd I understand your point of view. However, this is working as designed. Astro does not have an abstraction for As Astro does not attempt to solve the problem you are hoping to solve I would advise you participate in this roadmap issue which might solve the problem you're experiencing. In the meantime I would advise this: do not try to abstract all of your environment variables into a single file and use it in your config, your app, and in the client. Accept the fact that you're going to have some redundant code, as it's necessary at this time. |
Thanks also, Matthew, and fully understood I promise you. It's not fundamentally the redundancy, little as users here will like that; it's the risk. I've hit this block on 'we never intended to handle enviromentals otherwise' from others, but it seems to go deeper. That you've blocked off the vite.config.ts is what I really heard was center of intention now, according to this design rule, and that's where problems begin and multiply from. I remind that as far as I've seen, import.meta.env can't work with needed ENV_NAMES, unless loadEnv is used earlier, because Vite config is prevented from running early now. Result: integrations can't be configured. It's a loop all this runs in, and then you have the risks of blind server errors -- by any mistake in where client imports come from. I appreciate this is formidable, and that's why I've suggested another path is needed for what that problem one-liner in 4.5 does. There's probably more behind the intensity to employ it, however it was generated, I picked up from some early nice chat as it always is with Blu. But do you need this blanket block truly? Perhaps I could get into futures discussions, but the problem is now, thus asking to unwind it, as recent long experience hasn't shown whatever dangers there may be to show themselves -- and this change has done. Now, that walk :) , and truly thanks, to both of you. |
Ok, thanks for taking the time to have the conversation. Since we aren't going to restore loading the Vite config, I'm going to close this issue. I encourage you to participate in the astro:env RFC and hopefully that can help. |
Astro Info
If this issue only occurs in one browser, which browser is a problem?
any browser
Describe the Bug
I'm going to go step by step here, but you are welcome to skip to the Realistic Solution, not far below :)
How the problems show...
How can we resolve this?
I discovered two ways.
In the end, to prove out several points and isolate when a week's high effort disclosed nothing useful on any intuition from the bogus errors, I created an Astro 4.5.9 from a fork, obfuscated its identity to avoid issues none of us would like, and made one change: I reverted the source of all problems, the line inserted into the 4.5 build from a worthy attempt to fix an obscure problem, whose actual conversation is here:, and published this, also obscurely.
Using this single reversion, my complex app returned to working entirely properly, operating Sanity's very interprocess-entwined Presentation edit-from-web-page technology perfectly -- and using only the desirable import.meta.env.
I had in fact finally made the situation operate with Astro's current own deployment.
But what it took, after working through all the erroneous messaging and then obscure clue-hunting and easter-egging piece-by-piece into realizing the nature of the failure and causes, never visible itself...
...first of all, isolating all uses of environmental variables, some of which are intertwined and complicated, into two entire copies. One for the server side uses, employint that loadEnv which forces this, and the second copy employing import.meta.env, for all the client-side use equally requiring that, some of which are rather obscure, as they occur inside components non too simple, passed by props.
It's important to note that none of the client-side variables could operate unless I opened loadEnv to import 'everything', thus allowing import.meta.env to see them. Thus all safety issues with not prefixing environmentals were opened occur. That's because the reverted code change prevents setting the envPrefix which properly governs this for import.meta.env.
Finally, all my Vite config had to be moved inside of astro.config.ts, since vite.config.ts was ignored, making that far more complicated than I want it to be, for the developer/appbuilder-cultured persons this Astro-Sanity easy-onramps, full Visual Edit solution is intended to be for.
Understanding the Framing
Honestly, I can't think the 'just use loadEnv' advice is a solution whose results Astro would want to have occur, in any way. If you peruse some of the false error screens and quotes in prior versions of this issue, I think you will agree this is nothing a using person should be confronted with. Indeed, I found lonely references to some of them on the web -- plaintive and never with an answer, because who could give one?
Further, when I look at the actual issue the 4.5-included problem change attempts to solve, it is quite obscure, itself -- occurring in a Vite custom plugin, and really sounding like it needs to be fixed at its source. The repair attempt was by someone who admitted they hadn't fully understood. I had some side conversation where it was suggested this 'fix' was to cure general risks in Astro's use of Vite, but I don't see this in the problem.
A Realistic Solution
I would propose, then, respecting all possibilities, including the open-ended one -- but beginning,in practical mind, with full repair very much in the present:
first of all, revert the line ending use of vite.config.ts in a next most minor release of Astro 4.5 , now and forward.
It cannot be worth the pain shown above, and the risks to Astro's future in opportunities, to leave it there with its consequences. Let Astro apps continue work as they have been, be configured as they have been.
then, create a healthy, accurately focused solution for the issue that drove this problem change. Focus it on exactly what occurred, likely by filtering in the Astro code which gets involved as shown with the original issue.
if indeed the original problem hinted into further issues, prevent those in the same fashion, with focus and accuracy.
I'd feel this solution set would live up to the excellence which Astro's premise, and the continuously evidenced design and architectural creativities of its core team, have always shown. And it's what your public presentation and those guiding it would like to live and be so lively, isn't it so?
Thank you for listening.
What's the expected result?
Expected would be that complex, environmental variable-dependent applications would continue to work flawlessly, wherever developed or deployed, in Astro as it moves to 4.5.x
[n.b. the minimal example that follows only shows the initial problem, not the waterfall of difficulty that results as you try to solve it....]
Link to Minimal Reproducible Example
https://stackblitz.com/~/github.com/narration-sd/github-lerf2k-suvouc
Participation
The text was updated successfully, but these errors were encountered: