stencil icon indicating copy to clipboard operation
stencil copied to clipboard

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

Open weaintplastic opened this issue 2 years ago • 17 comments

Prerequisites

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 avatar Apr 07 '22 08:04 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 avatar Apr 15 '22 08:04 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.

weaintplastic avatar May 12 '22 06:05 weaintplastic

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.

rwaskiewicz avatar May 12 '22 13:05 rwaskiewicz

@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.

image

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

weaintplastic avatar Jun 28 '22 08:06 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 avatar Aug 11 '22 10:08 weaintplastic

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

weaintplastic avatar Feb 10 '23 07:02 weaintplastic

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

johnjenkins avatar Feb 10 '23 07:02 johnjenkins

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?

mhoritani avatar Jul 28 '23 19:07 mhoritani

@rwaskiewicz Can you please look into this??

sgr-kumar avatar Jan 22 '24 06:01 sgr-kumar

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!

rwaskiewicz avatar Jan 22 '24 13:01 rwaskiewicz

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

mhoritani avatar Jan 22 '24 14:01 mhoritani

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

sgr-kumar avatar Jan 30 '24 10:01 sgr-kumar

Hi @rwaskiewicz , We found a potential fix for our issue and I opened up a PR: https://github.com/ionic-team/stencil/pull/5317 We would welcome any assistance or suggestions to merge this into the main 🙏

andrew9994 avatar Feb 01 '24 10:02 andrew9994

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!

tanner-reits avatar Feb 01 '24 21:02 tanner-reits

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?

andrew9994 avatar Feb 01 '24 22:02 andrew9994

@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!

tanner-reits avatar Feb 02 '24 21:02 tanner-reits

@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.

andrew9994 avatar Feb 04 '24 21:02 andrew9994

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

alicewriteswrongs avatar Mar 04 '24 18:03 alicewriteswrongs