Replies: 26 comments 3 replies
-
Thanks for diving into this! I'd love to improve things here, but I have a few questions.
Would it make sense to add this API to the testing tool as a placeholder method? I'm sure I'm not the only one using this, and I'm fine with using optional chains if needed, but I'm concerned that contributors and even myself will remove the I can't really think of a great way to prevent that from happening in the code (short of specific comments or tests), so it would feel a lot better if the testing tool implemented it as a no-op to prevent fails. Maybe it can emit a warning if there's a potential pitfall in relying on it? 🤔
I'm OK with deferring this one. Great suggestion!
I'd like to reuse the parser to avoid the overhead of instantiating a new one for each icon. Is it safe to move it to the constructor, or will that also cause the test tool to fail?
This is indeed unfortunate. I'd really like to see a CSS solution that solves for whitespace and the absence or presence of text nodes, element nodes, etc. since the browser already has that information. I can see if there are any instances that can be solved with pure CSS, but Thanks again! |
Beta Was this translation helpful? Give feedback.
-
One more thing...is there a list of issues/gotchas that I can reference? Knowing what not to do will be helpful, and I'd also like to link to or add it to the contribution guidelines. |
Beta Was this translation helpful? Give feedback.
-
We could potentially add a no-op
The constructor is also run server side where
We have a brief documentation of SSR limitations here https://github.com/lit/lit/tree/main/packages/labs/ssr#notes-and-limitations. We're working on expanding this more hopefully soon! |
Beta Was this translation helpful? Give feedback.
-
I'd love to find a way to SSR the icons, not just delay their parsing/render till hydration. That would probably require quite a change in the icon system though. Some of on the Lit team may go off an experiment there and see what we can come up with... |
Beta Was this translation helpful? Give feedback.
-
I suppose we could create a new instance on I can do this in the meantime. I’m interested to see what Justin et al come up with, but I’m not sure how it will work without baking in all the icons. I’d really like to continue supporting multiple icon libs that fetch from anywhere. |
Beta Was this translation helpful? Give feedback.
-
Updates:
Outstanding issues include:
|
Beta Was this translation helpful? Give feedback.
-
Today's release includes the fix for base path and DOMParser. I'm willing to solve the |
Beta Was this translation helpful? Give feedback.
-
Bumping this to see if we can address this one:
Also, is anyone of aware of a CSS proposal that would let us remove slot detection logic? Should we start one? |
Beta Was this translation helpful? Give feedback.
-
I'll bring up adding a no-op For the CSS proposal, this one seems most relevant. WICG/webcomponents#936 |
Beta Was this translation helpful? Give feedback.
-
@claviska Now that Safari supports focus-visible out of the box, you can probably remove the feature detection alltogether. https://caniuse.com/?search=focus-visible |
Beta Was this translation helpful? Give feedback.
-
Yes, now that the last two major versions (15 + 16) support it, the shim has been removed. |
Beta Was this translation helpful? Give feedback.
-
So we are moving in a direction to reduce the reliance on these no-op DOM shims as we can't account for what the correct intention of the shim's use case would be, if no-op should even be the right thing to do. It would be better for authors to be able to handle these more intentionally so we're looking to provide a way to do that here lit/lit#3158. |
Beta Was this translation helpful? Give feedback.
-
Subscribed, thanks. I like this idea since it would give authors the ability to render a fallback or skip certain operations as needed. While not ideal, this is better than SSR failing and I don't think we're going to get a usable CSS solution anytime soon. |
Beta Was this translation helpful? Give feedback.
-
For slot selection, since we don't have a proper CSS solution right now, what if we ask users that want to support SSR with no FOUC to manually say whether or not the element has a slot etc. For example sl-select: export default class SlSelect extends ShoelaceElement {
...
private readonly hasSlotController: HasSlotController|null = null;
...
@property({type:boolean, attribute: 'has-label'}) hasLabel = false;
@property({type:boolean, attribute: 'has-help-text'}) hasHelpText = false;
...
// runs only on client and before first render so no FOUC if not SSR
update(changed: PropertyValues<this>) {
if (!this.hasSlotController) {
this.hasSlotController = new HasSlotController(this, 'help-text', 'label');
}
super.update(changed);
}
...
render() {
// on server: uses the host property
// on client: hasSlotController should be ready by render and update accordingly
const hasLabelSlot = this.hasSlotController?.test?.('label') ?? this.hasLabel;
const hasHelpTextSlot = this.hasSlotController?.test?.('help-text') ?? this.hasHelpText;
...
}
...
} SSR UsersThis will allow shoelace to be SSRd, and once hydrated on the client, Normal PeopleIf the user doesn't care about SSR, then their component should function exactly the same under the hood with negligible performance overhead from checking some if statements. DXDX can probably be cleaned up for the render method by making a util function. Update can probably be cleaned up in a mixin or in The futureThis can work as a workaround until we get a proper CSS solution for this. When we do get a proper CSS solution in enough browser coverage, you'd have to strike a breaking version change to remove the added properties and replace both this workaround + |
Beta Was this translation helpful? Give feedback.
-
The footprint is my biggest concern, but creating a convention might reduce it and make it more mixin-friendly. What about something like this? <sl-card ssr-slots="header footer image"> I can probably add It would need to support default slots too, which are currently represented by <sl-card ssr-slots="[default] header footer image"> Does this seem like a reasonable solution? Are there any other things we should be considering in terms of SSR, or is slot detection the last big blocker? |
Beta Was this translation helpful? Give feedback.
-
I love the idea of using // in the mixin
@property({attribute: 'ssr-slots', converter: {
fromAttribute: val => {
return val.split(' ');
},
toAttribute: val => {
return val.join(' ');
}
}})
ssrSlots = [];
protected readonly hasSlotController: HasSlotController|null = null;
update(changed: Map<string, unknown>) {
this.hasSlotController = new HasSlotController(this, ...this.ssrSlots);
super.update(changed);
}
protected slotTest(value: string) {
return this.hasSlotController?.test?.(value) ?? this.ssrSlots.includes(value);
}
// in the actual code
export class Select extends SsrSlotMixin(SlElement) {
...
render() {
const hasLabelSlot = this.slotTest('label');
const hasDefaultSlot = this.slotTest('[default]');
... In terms of other missing things I'm not totally sure because one error might be hiding another one. I'll take a look at seeing if I can set something sharable up in the coming days |
Beta Was this translation helpful? Give feedback.
-
To update: all of the aforementioned issues have been resolved now except for slot detection. What would be the best "standard" way to move forward with an alternative for slot detection? Before we do custom things, is there any emerging best practice for doing this in Lit or other libraries? |
Beta Was this translation helpful? Give feedback.
-
Awesome news! For slot detection, I think we should investigate whether Lit can support something directly. I don't think we'd want to support But we might be able to add SSR-specific code to something like the SlotController RFC so that it's We also have to consider rendering order to continue to support streaming. Reading slot contents requires rendering children, which usually wouldn't have happened until after the host's shadow root is rendered, so we'd have to buffer the children to stream out later. cc @aomarks @augustjk @sorvell and @kevinpschaaf from the Lit team. |
Beta Was this translation helpful? Give feedback.
-
Two strategies are currently available to work around imperative slot detection: User-provided AttributesThe user provides attributes indicating which slots have been populated. This is extra work for end users, as they have to slot the content and add an attribute. If we decide to to this as a stopgap, I'd prefer to use a single attribute instead of littering the API with multiple properties. Perhaps something like this. <sl-button variant="default" has-slots="prefix suffix">
<sl-icon slot="prefix" name="link-45deg"></sl-icon>
<sl-icon slot="suffix" name="box-arrow-up-right"></sl-icon>
Open
</sl-button> I'd also prefer to keep the Collapsible ContainersInstead of programmatically showing/hiding containers based on the presence of a slot, we bear the burden of removing the slot detection controller in favor of containers and styles that effectively collapse the container when no content is slotted. This might not be as easy as it sounds for all existing components, but it's worth exploring. |
Beta Was this translation helpful? Give feedback.
-
presented update proposal to WCCG for TPAC prep. Here is my current draft: TL;DR, I think it would be good to help advance the we would be able to do things like so: .label:has(~ .icon /slotted/ *) {
/* label icons when only icon sibling has slotted nodes */
} please leave comments on that discussion for what you would like for me to add to the status update to TPAC browser implementers. UPDATE: Browser implementers at TPAC very much disliked |
Beta Was this translation helpful? Give feedback.
-
I'm out of the loop, but why do they prefer a new syntax over a more common pseudo selector? It seems like things will get confusing, especially since we already have |
Beta Was this translation helpful? Give feedback.
-
it's honestly an ambitious proposal, but But the benefits are that it solves a lot of issues that
It gives a unified approach to shadow dom selectors that are more inline with selectors outside of shadow DOM (one less shadow dom smell and not "just-another-shadow-dom-thing") and it fixes more issues around slotting. The "what would happen to |
Beta Was this translation helpful? Give feedback.
-
Ahh, thanks for the explanation. I wasn't aware of the new syntax and assumed it was made for this purpose alone. I like everything I'm hearing, though. And it makes sense that Thanks again! |
Beta Was this translation helpful? Give feedback.
-
@e111077 That's a great explanation, thank you. |
Beta Was this translation helpful? Give feedback.
-
Any updates on what's holding up SSR for Shoelace? I tried building in Astro and ran into a bunch of DOM stuff |
Beta Was this translation helpful? Give feedback.
-
Was also wondering when SSR will be implemented given Lit now provides SSR support and a simple polyfil for Declarative Shadow DOM on the client. |
Beta Was this translation helpful? Give feedback.
-
Hello!
I've been testing out a work in progress utility for testing Lit elements being rendered server-side (lit/lit#2957) and have been trying it out on some shoelace component tests. I wanted to report some initial findings for things that were breaking when putting the components through lit-ssr which were mostly due to unavailable DOM APIs.
focus-visible check
shoelace/src/internal/focus-visible.ts
Lines 6 to 21 in 2157f4a
style.remove()
in the finally block.style.remove?.()
lets this proceed without throwing and I believe the check runs properly when styles are updated during hydration.base-path
shoelace/src/utilities/base-path.ts
Line 28 in 2157f4a
getElementByTagName
getBasePath
is called would be okay?icon.ts using DOMParser
shoelace/src/components/icon/icon.ts
Line 11 in 2157f4a
hasSlotController
shoelace/src/internal/slot.ts
Lines 38 to 40 in 2157f4a
render
during SSR,this.host.querySelector
doesn't exist.I'll add more findings as I come across them. Thanks!
Beta Was this translation helpful? Give feedback.
All reactions