prashantpalikhe / nuxt-ssr-lit

SSR support for Lit elements in Nuxt3
Other
47 stars 7 forks source link

Hydration is incorrect as some attributes are missing in the SSR HTML #34

Closed steveworkman closed 3 weeks ago

steveworkman commented 1 year ago

Describe the bug

Given a simple element with a label

<my-button label="one"></my-button>

This renders on the server without the label attribute as it's been set as a DOM property.

When hydration comes around client-side, the component does not know what the value of label is as it's not been set as an attribute on the HTML

Expected behavior

Attributes needed for hydration should be rendered in the HTML response

Additional context

This could be a component issue, as setting a property to reflect: true resolves this for that component. However, I don't think that's reasonable for things like a label on a button. There's a lot of information in https://lit.dev/docs/components/properties/ but it's not clear what should happen with SSR.

steveworkman commented 1 year ago

I believe we can make it a little better by doing something like this

attachPropsToRenderer(): void {
      const customElementConstructor = getCustomElementConstructor(this.litElementTagName);
      const props = this.litElementVnode.props;

      if (props && this.renderer) {
        for (const [key, value] of Object.entries(props)) {
          // check if this is a reactive property
          if (
            customElementConstructor !== null && // The element exists
            typeof customElementConstructor !== "string" && // It's not just a string
            key in customElementConstructor.prototype && // The property we're looking for is on the prototype
            ["boolean", "function", "object"].includes(typeof value) // It's a value that doesn't render as an attribute
          ) {
            this.renderer.setProperty(key, value);
          } else {
            this.renderer.setAttribute(key, value);
          }
        }
      }
    }

This solves the basic issue, and the accordion component in the playground does not need to be reflected any more, but I'm not sure it's perfect.

augustjk commented 1 year ago

I don't think the typeof check is necessarily the right thing. Rather, if the key is not in the element prototype, it's not something the element is expecting to get as a prop so it should be set as attribute, perhaps with String(value).

Maybe it's also a matter of how Nuxt handles props/attributes for custom elements. I see only attributes are added as props here, would it make sense to pass in all props here? https://github.com/prashantpalikhe/nuxt-ssr-lit/blob/4b72344a1c975a43683f6046f83777b5192f50db/src/runtime/components/LitWrapperServer.ts#L109-L115

steveworkman commented 1 year ago

I don't think the typeof check is necessarily the right thing. Rather, if the key is not in the element prototype, it's not something the element is expecting to get as a prop so it should be set as attribute, perhaps with String(value).

Maybe it's also a matter of how Nuxt handles props/attributes for custom elements. I see only attributes are added as props here, would it make sense to pass in all props here?

https://github.com/prashantpalikhe/nuxt-ssr-lit/blob/4b72344a1c975a43683f6046f83777b5192f50db/src/runtime/components/LitWrapperServer.ts#L109-L115

This broadly seems to work in the cases we have so far. I'm going to add more tests to see if I can cover the range of what is going on

prashantpalikhe commented 1 year ago

I think rendering out properties (that may or may not be reflected in attributes) as attributes always may cause issues related to #30

I think the underlying problem here is that the client-side render of the Lit component seems to happen incorrectly. When Vue/Nuxt renders the component like <my-button label="one"></my-button> on the client, it should set the label property to the web component correctly (I think this is not happening for an unknown reason). If that is done correctly, then Lit's hydrator should have the correct render result when it invokes the render on the patched Lit component.

@augustjk correct me if my hunch is wrong here

augustjk commented 1 year ago

Hmm.. I think in principle that makes sense. For server rendering, setting property on the element only makes sense for generating the shadow dom, and the generated markup of the element should just contain the attributes, including any reflected properties.

As @prashantpalikhe pointed out, the problem could lie more in how vdom node on the client side maps to the custom element. We did notice the vdom node did have all the props but wasn't setting them on the actual custom element on the client once loaded. Perhaps the difference in client/server behavior of the wrapper confuses Vue/Nuxt that it doesn't know the custom element generated from the h(this.litElementTagName, ...) is meant to be the target to add props to?

steveworkman commented 1 year ago

I've added a test component based on one we have at Maersk that is showing the hydration and SSR issue in https://github.com/prashantpalikhe/nuxt-ssr-lit/pull/35 (my-step-indicator).

The outer component, which was wrapped in the LitWrapper is rendered, but the inner component does not have its shadowroot filled, despite the render method being called with the correct properties. I'm not sure if this is an issue with the lit renderer or the way we're passing properties, but it is showing up as a hydration issue int he console.

The first error says there should be exactly one root part in a render container but in this case there are zero elements. There are three of them as we are trying to render three<my-step-indicator-item> elements

The second error message is a side-effect of this as it's trying to hydrate something that doesn't match.

image

augustjk commented 1 year ago

The hydration error I think steps from the conditional render in my-step-indicator. If this.labels might have the 3 items in one environment and defaulting to empty array in another environment. Does the server rendered declarative shadow dom contain the 3 items? If so, then it might be Vue failing to pass the props to the correct element before hydration.

steveworkman commented 1 year ago

@augustjk yes, in this case labels is [] client-side - it's passing through perfectly well server-side. Changing the property to a string and splitting it with commas works without any issue. It also looks like it is specifically Arrays - I was playing with it and found that passing in an object of { labels: ['label'] } passed the outer object through, but not the Array!

augustjk commented 1 year ago

Is it specifically Vue/Nuxt not being able to provide array props to the element? That's odd!

prashantpalikhe commented 1 year ago

When looking a bit into the hydration issue earlier, I had noticed a strange situation where the custom element was instantiated was Vue and therefore passed in all the props while NOT using the SSR. But while using the SSR, the custom element was instantiated as soon as the custom element definition arrived on the browser. And the call stack showed nothing else. The first call in the call stack was customElement.define(...). So the custom element got instantiated with no props. That maybe the cause of this issue. No idea yet why it behaved differently. Maybe presence of the DSD?

UPDATE: This was not related to the issue. But did help me eventually get in the right direction. Check my comment below for the newer findings.

prashantpalikhe commented 1 year ago

@steveworkman @augustjk

So I dived into this further, investigating how Vue applies props to the custom elements while hydrating an SSR-ed application. And I found out that Vue does not apply props to components during hydration.

https://github.com/vuejs/core/blob/9698dd3cf1dfdb95d4dc4b4f7bd24ff94b4b5d84/packages/runtime-core/src/hydration.ts#L329

You can see here that only props named value or starting with on are applied/patched.

IMO, this is the reason why we noticed that during Lit's hydration, the Lit components were instantiated without correct values (only default values).

We got around this issue by rendering all props as attributes. But I think that's not always desirable (explicitly set by component authors to not reflect as an attribute) and in the case of complex data types, like array/objects, not possible either.

But if we were to apply the props for custom elements during hydration on Vue's side, then the issue gets resolved.

We do need to defer Lit's hydration until Vue's hydration is complete. But that's very simple to achieve by adding a defer-hydration attribute in the SSR-ed Lit component and removing it when Vue mounts the Lit component on the client side.

I tried @steveworkman 's branch by making these modifications, including patching Vue's source code to apply props for custom elements during hydration, and the issue is resolved. Would be great to discuss this further with a Vue dev. to get their insight into this.

prashantpalikhe commented 3 weeks ago

@steveworkman I think this issue can be closed now, now that Vue has fixed the bug, right?

steveworkman commented 3 weeks ago

Yes, fixed in 3.4.36