-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: v2 backpatch support #7900
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
Conversation
🦋 Changeset detectedLatest commit: 9ffb42d 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 |
commit: |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
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 is excellent! Some small nitpicks.
Also, this looks like a good start for OOS, but at that point there might be a noticeable delay between DOM content visible on screen and end of the document arriving in the browser.
How should the backpatcher be changed to allow patching while streaming? Maybe emit the backpatcher as soon as it's needed and then emit script tags with patches every 50ms or so?
That is assuming you can change DOM while it's still streaming in 🤔
Yes there would definitely be a noticeable delay in that case. There is only one data script per container rather than mini scoped ones. This is because the executor script runs at the end anyways and you can improve the encoding of the data script a ton, especially with multiple component instances. I definitely see it as a separate thing. We should be able to see from the scheduler if the consumer clearly intends to be doing OOO Streaming, in that case we can then emit the script tags / wrapper approach as we go. In Marko 6 they opt-in by providing fallback UI. |
'@qwik.dev/core': patch | ||
--- | ||
|
||
feat: add SSR backpatching (attributes-only) to ensure SSR/CSR parity for signal-driven attributes; limited to attribute updates (not OoO streaming) |
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.
Can you explain the use-case a little bit better for qwik users? e.g. when is it necessary, how it works, what needs to be done to use it (as an overview ofc)
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.
hm... I could try from a high level, although that is a lot to put in a changelog
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.
Suggestion:
feat: add SSR backpatching (attributes-only) to allow parent components to know some information about their children during SSR: like how many children they have, or if a certain child contains an attribute or not... This was already achievable in CSR, and is now possible in SSR too.
|
||
This creates problems for component libraries where elements need to reference each other (like form inputs linking to their labels via `aria-labelledby`). If the input streams before its label, it can't know the label's ID to set the relationship. | ||
|
||
Backpatching automatically fixes these relationships by updating attributes after the entire page has streamed, giving you the same flexibility as client-side rendering. |
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.
Maybe add a sentence to say something along the lines of "as a developer, you don't need to do anything special. Qwik will automatically detect if it needs to backpatch a certain attribute when necessary".
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.
yeah I can add that in
|
||
> Note: This is not Out-of-Order Streaming. It only corrects already-sent attributes without delaying the stream. | ||
### Example |
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.
Is this "Example without backpatching"?
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.
no
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.
Then I don't get it. It seemed to me the example shows the problem before the PR. After the PR it will just work right? Better to make that more explicit imo.
What is it?
Description
This PR implements automatic SSR backpatching, enabling SSR-correct relationships between components whose render order can't be guaranteed. Unlike traditional SSR where once HTML is sent you can't modify it, backpatching automatically corrects attribute relationships after streaming completes.
Checklist
pnpm change