-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix(NodeApp): fix responses with null bodies never completing #9931
Conversation
🦋 Changeset detectedLatest commit: 61a606c The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@lilnasy I've come up with a different test implementation should i discard my changes or do you want to integrate them with your's? |
Do you mean for the tests? |
@lilnasy Yes correct, I mean the test implementation. |
The test looks great to me, I want to add that. Although adding a new fixture also means adding its build-time to the CI. If you could make it use an existing fixture, I will add it. |
@lilnasy The only other fixture with the "SSR" prefix that has "@astrojs/node" in it's dependencies is "ssr-api-route". Should I migrate the tests into this fixture then? |
Could you use the one used in the current test? |
@lilnasy Sure i'll use your implementation as a base then, give me ~5 minutes. |
@lilnasy It took me a bit more than 5 minutes, sorry for that. (I am not familiar with the Node.js test runner) |
I need to adjust errors.test.js. I can cherry pick your commit afterwards, thanks! |
Changes
Testing
Docs
Does not affect usage.