Skip to content
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

feat: don't hydrate class name in html tag anymore #6091

Closed
3 tasks done
christian-bromann opened this issue Jan 10, 2025 · 7 comments · Fixed by #6103
Closed
3 tasks done

feat: don't hydrate class name in html tag anymore #6091

christian-bromann opened this issue Jan 10, 2025 · 7 comments · Fixed by #6103
Assignees
Labels
Request For Comments Seeking commentary on an issue or PR from the community

Comments

@christian-bromann
Copy link
Member

christian-bromann commented Jan 10, 2025

Prerequisites

Describe the Feature Request

Currently Stencil sets a hydrate class name on the html tag do avoid flicker effects in Ionic applications. Ionic uses the following style to enable it:

html:not(.hydrated) body {
  display: none;
}

The problem with this approach is that Next.js applications throw an error as they interpret this as an unintentional.

Describe the Use Case

Especially when components are lazy loaded we have to ensure that applications can load smoothly.

Describe Preferred Solution

We should not block the application from being visible just because there are some Stencil components that take longer to load. I think the best approach here is to just remove this behavior from the appDidLoad function. However this may have an impact on the Ionic project which we need to evaluate.

Describe Alternatives

No response

Related Code

No response

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Jan 10, 2025
@christian-bromann christian-bromann added Request For Comments Seeking commentary on an issue or PR from the community Help Wanted and removed triage labels Jan 10, 2025
@danielleroux
Copy link
Contributor

danielleroux commented Jan 17, 2025

Would be a config flag a acceptable solution for this? In this case ionic can keep the default behaviour and all other projects can configure the components?

@christian-bromann
Copy link
Member Author

@danielleroux I am currently looking into this and have a fix hopefully by next week. We likely will disable this for output targets.

@christian-bromann
Copy link
Member Author

This has been released in @stencil/core@4.25.2

@cmaas
Copy link

cmaas commented Mar 10, 2025

If anyone is curious how to re-enable the old behavior:

window.addEventListener('appload', () => {
	document.getElementsByTagName('html')[0].classList.add('hydrated');
});

Because if you simply use Stencil and Ionic in a vanilla app, this "fix" is a breaking change.

@christian-bromann It would've been nice if this was marked as a breaking change and not as a "feature" in a patch version number without any instructions on how to revert this.

@christian-bromann
Copy link
Member Author

@cmaas we have been aware that this could eventually have implication on Ionic users. However we had no data about how many folks would be impacted, if any. With a project this size it is challenging to define what behavior is "officially" supported to allow categorizing it as a breaking change vs. a bug fix. Mind sharing a minimal example of the type of application you are building, maybe it helps to better understand why this was implemented in the first place.

@cmaas
Copy link

cmaas commented Mar 10, 2025

Thanks for the reply. I'm building a pretty standard Ionic app, built with Stencil, wrapped in Capacitor and distributed on Play and App Store. I've been using both Stencil, Ionic and Capacitor since the beginning more or less (~2017). I'm not sure when the hydrated class was added to the html tag. I guess it's been there forever. It seems to be in Stencil mostly to support Ionic, because the styling is defined in Ionic. If one doesn't use Ionic with Stencil, the hydrated class is probably useless.

With Ionic, I think it's basically default behavior. Stencil added the hydrated class by default, now it needs to be added manually. Otherwise everything is display:none.

My gut feeling is: it actually affects everybody building mobile apps with Ionic and Stencil, because the styles are defined in structure.css. See default app.css, which imports structure.css in the ionic starter: https://github.com/stencil-community/stencil-ionic-starter/blob/main/src/global/app.css

@duhem-s
Copy link

duhem-s commented Mar 19, 2025

Hi, I just looked into why renovate could not upgrade our Stencil version.

In our e2e tests, to ensure Stencil is properly loaded, we were waiting for the hydrated class: MGDIS/core-ui@4d1d89a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Request For Comments Seeking commentary on an issue or PR from the community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants