Conversation
|
|
|
So my test actually caused a fail, and I did saw that error happening locally once in a while. I try to check again that later tonight, or otherwise remove the test for now. |
patak-cat
left a comment
There was a problem hiding this comment.
Thanks for looking into this @bluwy! I have doubts if this is the best long-term solution. I'm afraid there could be some global state that gets corrupted because this error wasn't handled. With the browser we know there will be a hard reload but that isn't the case with ssrLoadModule. Should SvelteKit have a try/catch and ignore the error instead?
I think your solution is good for a change done in a minor though. So we better merge it and then we could review in Vite 5 if we should rework this.
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
|
Actually, yeah. I did this fix a bit too quick, it indeed seems like it would cause an incorrect state in SvelteKit side 🤔 Maybe we should fix it there instead. I thought fixing this here first because the error we emit is sort-of internal and it's meant for us to catch and not corrupt the state/server. Which in the issue's case it exits the server. (EDIT: after further checking, the exist could be cause by the hack instead.) |
|
Discussed with patak that this isn't the right fix as it returns an incorrect state. |
Description
fix #13735
The bug comes from this SvelteKit code, which runs
ssrLoadModuleimmediately on reload request.Additional context
It's hard to make the test error reliably, so added a very simple one for now. I noticed it may happen once in a while which we could catch.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).