withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
43.84k stars 2.28k forks source link

fix: prevent client hydration when rendering via Container API #11348

Open ematipico opened 2 days ago

ematipico commented 2 days ago

Changes

This PR fixes a bug inside the container API, where an error was thrown when rendering a client component using client:* directives.

This issue was brought up on discord: https://discord.com/channels/830184174198718474/1255408685275021342

To fix this bug, I did the two following things:

Testing

Added a new test case

Docs

N/A

changeset-bot[bot] commented 2 days ago

🦋 Changeset detected

Latest commit: 5113f56af14903e44fbb6d4773407169b11cdb8b

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

matthewp commented 2 days ago

Does this prevent using client directives in all Container rendered content or just some? If this prevents all then that means the PHP / Rails use-case doesn't work any more.

ematipico commented 2 days ago

Does this prevent using client directives in all Container rendered content or just some? If this prevents all then that means the PHP / Rails use-case doesn't work any more.

You're probably right. Do you think we should provide an option when we call the render function?

ematipico commented 1 day ago

I added an option to the render* functions.

matthewp commented 1 day ago

I don't think we should have such an option. Why is the user not passing through the directives? Is the problem that we don't have a good way to import those into the test? If so that's what we should fix, not add an option.

ematipico commented 1 day ago

I don't think we should have such an option. Why is the user not passing through the directives? Is the problem that we don't have a good way to import those into the test? If so that's what we should fix, not add an option.

Users have components like this:

---
// Button.astro
import VueButton from "../components/Button.vue"
---

<VueButton client:idle>Text<VueButton>

So, I am not sure there's a good way to "turn off" the hydration checks inside the renderer. Do you have any concrete solution in mind?

matthewp commented 1 day ago

You should be able to pass in the directive config via AstroContainer. It's on the SSR manifest. Can we expose this? https://github.com/withastro/astro/blob/main/packages/astro/src/core/client-directive/default.ts#L7

import {getDefaultClientDirectives, AstroContainer} from 'astro:container';

let container = await AstroContainer.create({
  clientDirectives: getDefaultClientDirectives()
});
ematipico commented 1 day ago

You should be able to pass in the directive config via AstroContainer. It's on the SSR manifest. Can we expose this? main/packages/astro/src/core/client-directive/default.ts#L7

import {getDefaultClientDirectives, AstroContainer} from 'astro:container';

let container = await AstroContainer.create({
  clientDirectives: getDefaultClientDirectives()
});

This is already present in the current PR, but it isn't enough to avoid errors.