bug: client-side hydration of pre-rendered web component is broken for bundles that use dist-custom-element

[weaintplastic]

Prerequisites

• I have read the Contributing Guidelines.
• I agree to follow the Code of Conduct.
• I have searched for existing issues that already report this problem, without success.

Stencil Version

2.15.0

Current Behavior

It seems that the client-side hydration of pre-rendered web components leads to different results when comparing the use of Stencil's dist bundle and a custom bundle based on dist-custom-elements output.

Pre-rendered HTML

<my-component class="sc-my-component-h hydrated" s-id="1">
  <!--r.1-->
  <!--o.0.1.-->
  <label class="sc-my-component sc-my-component-s" c-id="1.0.0.0">
    <!--s.1.1.1.0.-->
    <!--t.0.1-->
    This is the label
  </label>
</my-component>

After client-side hydration with Stencil's dist bundle, the html looks correct and is working as expected.

<my-component class="hydrated">
  <!-- #shadow-root (open) -->
  <label class="sc-my-component sc-my-component-s">
    <slot>
      <!-- ↳ #text reveal -->
    </slot>
  </label>
  <!---->
  This is the label
</my-component>

After client-side hydration with the custom app bundle that is using Stencil's dist-custom-elements distribution, the html looks very different due to the slotted content not being hydrated correctly.

<my-component class="sc-my-component-h hydrated" s-id="1">
  <!-- #shadow-root (open) -->
  <label>
    <slot>
      <!-- ↳ <label> reveal -->
    </slot>
  </label>
  <!--r.1-->
  <!--o.0.1.-->
  <label class="sc-my-component sc-my-component-s" c-id="1.0.0.0">
    <!--s.1.1.1.0.-->
    <!--t.0.1-->
    This is the label
  </label>
</my-component>

Expected Behavior

I'd expect the client-side hydrated results to be exactly the same and slots are resolved correctly.

Steps to Reproduce

I've set up a project to reproduce the bug in two different scenarios and included a thorough description on how to reproduce there.

https://github.com/weaintplastic/stencil-hydrate-bug-demo/blob/main/readme.md

Code Reproduction URL

https://github.com/weaintplastic/stencil-hydrate-bug-demo

Additional Information

I've been trying the same with the deprecated dist-custom-elements-bundle but facing the same issue there. So it is not a problem of dist-custom-elements output specifically.

[weaintplastic]

I'm more than happy to work on this bug.

Please let me know if you can point me to a good starting point for debugging this issue.

[weaintplastic]

👋 It's been a while since I've opened this ticket and I'd be more than grateful if someone could take a look at this quickly and point me in a direction. Thank you.

[rwaskiewicz]

Hey @weaintplastic - sorry about that! This somehow slipped through the cracks.

I'm going to label this to get it ingested into our internal backlog for someone on the team to take a closer look and get an idea for what's going on here.

[weaintplastic]

@rwaskiewicz Thanks for picking up this issue.

I've just upgraded the project to reproduce the bug to the latest version of Stencil 2.17.0
For better demonstration I've changed the component to render a <button> instead of a <label> so that the bug related to the revealed slots becomes more visible.

Again, I'm happy to support you fixing the bug if you can point me in a direction.

[weaintplastic]

Hello 👋. It's been a while and I wanted to check in if there is any news on this. If possible, you could give me a thought-starter on where to start debugging and I'll try my best to fix it.

[weaintplastic]

@rwaskiewicz do you have any news on this one? I would be happy to pick this up again.

[johnjenkins]

For coverage https://stencil-worldwide.slack.com/archives/C79EANFL7/p1675815754196299

[mhoritani]

This issue is still present in the latest version of Stencil (4.0.2 at the time of posting) and effectively breaks many components that have been rendered on the server with the hydrate app as soon as they are being reconciliated in the client.

Is there any way to get some traction on this, or can someone point us into the right direction so that we can look into it and potentially provide a fix?

[sgr-kumar]

@rwaskiewicz Can you please look into this??

[rwaskiewicz]

Hey folks,

Please do me a favor and add 👍's to the issue summary to upvote it instead of "+1" style comments or "at-ing" members of the team. GitHub doesn't give us an easy way to track those types of comments, which makes them more likely to not be properly counted when we prioritize issues.

Thanks!

[mhoritani]

We have done some research and found a potential fix for this. We are in the process of preparing another, more current reproduction, with findings and a proposal on how to fix it.

If you have any tips or guidance on how to best pack this up please let us know.

FYI: @weaintplastic @andrew9994

[sgr-kumar]

@mhoritani Is there any estimate or workaround for this issue??

[andrew9994]

Hi @rwaskiewicz ,
We found a potential fix for our issue and I opened up a PR: #5317
We would welcome any assistance or suggestions to merge this into the main 🙏

[tanner-reits]

Howdy @andrew9994, thanks for taking a stab at this! I was curious if there was a reason you (or anyone else running into this) want/need to consume the dist-custom-elements output rather than the lazy (dist) output? The Hydrate App was created to only leverage the lazy output, so we want to understand the use-case/need as we consider repercussions that may be triggered from this change. Thanks for your time!

[andrew9994]

Hey @tanner-reits , we have some apps which handle the bundling/tree-shaking and registration of the components themselves. That's why we are utilizing the dist-custom-elements output. Additionally, we would like to take advantage of the hydrate app for improved performance/accessibility/SEO. Could you please provide more details on the potential repercussions of this change? During testing, I couldn't identify any breaking changes, and I would appreciate guidance on this. Should we consider creating a new hydrate script tailored for the usage of dist-custom-elements instead?

[tanner-reits]

@andrew9994 We haven't encountered any repercussions in the brief testing we've done so far. We were just trying to gain some context and cover our bases as we look at the changes. So thanks for providing that! I don't think we need to go as far as a separate hydrate app, so no worries there.

I'll bring this back to the team for some internal discussion and we'll do some more testing/digging. We'll post any follow-up questions or discussions here! Thanks for your patience and contribution!

[andrew9994]

@tanner-reits, cool, thanks for the heads-up! Let me know if there is anything I can do to help your team move forward with this.

[alicewriteswrongs]

The fix for this was published in today's release!