-
Notifications
You must be signed in to change notification settings - Fork 25
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
RFC: Stylesheet deduplication for SSR #90
base: master
Are you sure you want to change the base?
Conversation
Feedback from @InstyleVII:
|
Feedback from @divmain:
For flexibility basically – consumers will probably use a hash, but they may have complex paths depending on CDN etc. |
TODO: need benchmark for light DOM as well. |
|
||
## Drawbacks | ||
|
||
This solution will add significant complexity both to the LWC engine (which will have to deal with supporting the new API in SSR, as well as dealing with hydration and mismatch detection on the client) and on the consumer side (which will have to do the bulk of the work of managing and hosting the stylesheets themselves). |
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 why this change increases the possibility of hydration mismatch?
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.
Right now the hydration mechanism for <style>
s is the same as any other DOM element. Whereas if we render <link>
s then we will need to instruct hydration with something like "If you see a <style>
with such-and-such text content (e.g. a checksum), then consider it the same as a <link>
with such-and-such attributes."
|
||
## Drawbacks | ||
|
||
This solution will add significant complexity both to the LWC engine (which will have to deal with supporting the new API in SSR, as well as potentially deduplicating styles client-side w.r.t the current CSR implementation) and on the consumer side (which will have to do the bulk of the work of managing and hosting the stylesheets themselves). |
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.
consumer side (which will have to do the bulk of the work of managing and hosting the stylesheets themselves)
Does the consumer need to add <style>
or <link>
tags to the base doc for each stylesheetCache
item? Or does the <lwc-style>
component handle the stylesheets (as long as registerLwcStyleComponent()
is called on the client)?
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, the consumer does not need to do anything. LWC itself (and notably the <lwc-style>
component) will 1) render a <style>
the first time it sees it, and put it in a global cache, and then 2) render nothing the next time, because it can look up the <style>
via the style-id
attribute. I'd recommend reading Rob Eisenberg's design which I shamelessly stole.
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.
on the consumer side (which will have to do the bulk of the work of managing and hosting the stylesheets themselves).
I am confused about this statement relative to the above comments.
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.
+1. That statement triggered my original question in this thread.
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.
Invalid sentence from the previous RFC. Removed.
```js | ||
import { registerLwcStyleComponent } from '@lwc/ssr-runtime' | ||
registerLwcStyleComponent() | ||
``` |
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.
What happens if registerLwcStyleComponent()
is not called? Will all SSRed components be un-styled?
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.
Correct.
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.
Should this call occur before hydrateComponent
?
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.
For sure, yes. It should happen as early as possible (to avoid FOUC).
|
||
The above API allows the caller of `renderComponent` to specify a `Set` to be used for deduplicating styles. This `Set` can be pre-populated (e.g. across renders of multiple islands of components), or created fresh. | ||
|
||
After `renderComponent` is called, the `stylesheetCache` (if specified) will be populated with any stylesheets (as strings) that were encountered during 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.
will be populated with any stylesheets (as strings)
Do you have an example of how the cache is pre-populated? It's not clear to me how the caller could pre-populate the cache.
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.
My idea is that the caller would normally provide an empty Set
. However, in the case of LWR (for example), you might call renderComponent
multiple times for multiple islands on the same page. However, these islands can still all share the same cache. (No sense in rendering the same <style>
once per island.)
So for example you could do:
const stylesheetCache = new Set()
for (const island of islands) {
await renderComponent(island, { stylesheetCache })
}
Co-authored-by: Khang Nguyen <[email protected]>
Rendered