-
-
Notifications
You must be signed in to change notification settings - Fork 3.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
test(): add SSR test #9490
base: master
Are you sure you want to change the base?
test(): add SSR test #9490
Conversation
Build Stats
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready to merge, might seem a lot of changed files but most are minor patches
e2e/tests/SSR/index.spec.ts
Outdated
|
||
test.beforeAll(({}, testInfo) => { | ||
testInfo.setTimeout(60 * 1000); | ||
task = exec(`npm start next -- -p ${PORT} --no-watch`, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once playwright/fabric are mjs then we can import startSandbox
directly instead of spawning this cmd
e2e/tests/SSR/index.spec.ts
Outdated
await page.getByRole('textbox').press('Control+a'); | ||
await page | ||
.getByRole('textbox') | ||
.fill("fabric supports SSR!\nIsn't that amazing?!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This message is confusing.
What fabric support is only not blowing up node with an import that run browser code.
How do we ensure that the page was actually rendered by server?
Imho the test can just curl the output of http://localhost:${PORT}/ and verify it contains the snapshot of html.
The fact that the page load could also simply mean that SSR isn't active, no?
I would also change fabric supports SSR!\nIsn't that amazing?!
in a softer a page that uses fabricJS will render ok with framework that use SSR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we ensure that the page was actually rendered by server?
It doesn't, does it?
That is truly supporting SSR.
It is confusing saying the current state is SSR support, you are right.
I mean you can import fabric in SSR and it will not break. It will run on the browser after dehydration.
It is interesting to support SSR by rendering on node and sending it as an image or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the test I just want to prove fabric works after window loads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
applied minors
fix parcel ts bug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed a parcel bug and rebased
this is good to go and should merge ASAP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixes parcel class bug
parcel-bundler/parcel#839 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few more crucial CD fixes
@@ -0,0 +1 @@ | |||
FROM node:18 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixes deployed sandbox node version
our package.json require >16.20 I think so this fixes that
codesandbox/codesandbox-client#1469 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
finished
Motivation
The recent SSR discussions and bugs related to env convinced me we need a test that shows SSR works so we can change env stuff with confident
Description
I created a test using the next template
Changes
I had to make the next template emit a signal so the test knows when the server/app is ready so I added logic to the template (instrumentation hook)
I adapted the cli start cmd by adding flags so that the browser and vscode launch only if flagged + added a port flag.
Added
tsconfig
to the vanilla app to fix a parcel ts bugFixed node template so that after deploying it works (node version mismatch)
Gist
In Action