Allow islands to be re-rendered with new props on page transition

[matthewp]

Demo1.mp4

Changes

• Copies over an islands props that are preserved via `transition:persist
• Fixes the React renderer so it doesn't create a new root when one exists, allowing state to be preserved.

Testing

• Added a test case

Docs

• Explain that props are updated on navigation docs#6915

[changeset-bot]

🦋 Changeset detected

Latest commit: 4a99bab

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

[github-actions]

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

[martrapp]

Wow @matthewp, that looks really cool! Preserved state and updated props!

Three questions came to my mind:

• If we update the properties of persisted island with the values of the new page, shouldn't we offer something similar for other persisted (non-island) elements, e.g. have a way to update their attributes with the values from the new page?
• Do we need a way to opt-out (or explicitly opt-in)?
• Have you had a chance to review the other UI frameworks? Are they robust to hydration errors?

[matthewp]

@martrapp I think for non-islands it's a bit trickier to determine what to do. For example, on a <video> you would not want to just copy over the src attribute because even if it is the same value, it would trigger the video to replay. So you'd have to do "set if not the same". And even then, what is the intent exactly? If two videos are different and you use persist, is the intent to change the video or keep the current one? I'm not sure.

This is true of islands to, of course. My assumption here is that islands are more adept to respond to the changes and keep the current state if that's the right thing to do. So I do think we should do a preview release here and have some people use it to see if it matches their expectations or not. We could provide an option in / out directive in either case.

[martrapp]

Bad example :), but i know what you mean

Dynamically modifying a source element's src or type attribute when the element is already inserted in a video or audio element will have no effect.

A nice example for non-islands would be updating the class attribute as transition:persist copies over the element with the current class attribute, but not the CSS referenced from the class attribute. Updating the class attribute with the values from the next page would guarantee that the referenced classes are found in the style sheets of the next page.

Of course, this can all be done using lifecycle events and no need to include it in the core implementation.

But there may be a point to give users some control over whether their island props being updated automatically ;-)

[matthewp]

@martrapp What about something like:

• transition:persist-props for islands. This defaults to false and props are updated. You can add this attribute to keep the props.
• transition:persist-attrs for non-islands (any regular element). This defaults to true. If you want to swap attributes use transition:persist-attrs="false"

Not sold on this tbh, just getting the conversation moving.

[martrapp]

Hey @matthewp

What about something like ...

Yes, great!

• transition:persist-props: This allows users to correct the default if it doesn't suit them.
• transition:persist-attrs: We can do this, but we don't have to. We haven't had any requests for updating attributes from outside so far. I just thought it made sense to discuss it when we were looking at the properties of islands.

[matthewp]

@martrapp Ok, I'll update the PR to implement transition:persist-props. I'll like the attr version alone for now, if we get requests it should be easy enough to add.

[martrapp]

if we get requests it should be easy enough to add.

Sounds good to me!

[sarah11918]

Hi @matthewp ! Following the conversation in this PR, I also wanted to add mention here of transition:persist-props as I think that's important to call out!

Also, the PR title says "islands" which we've always taken to mean a UI framework component. (e.g. not an Astro component) but doesn't mean our audience is necessarily going to have that distinction top of mind.

This changeset description uses "island" and "component" interchangeably, and no where screams "YOU CAN'T USE THIS ON AN ASTRO COMPONENT". I didn't add anything to that effect, but it could be worth a stronger, explicit mention somewhere if that's the case. It can be as simple as updating a couple of the times where the word "component" is used to "framework component" for example. So, you can decide whether you think that's necessary.

So, see what you think of my suggestions for your consideration!

[sarah11918]

Approving for docs, so it's clear!

Merge day

[Fakerko]

I think it works differently now than described in the documentation.

transition:persist-props

The documentation says:

This allows you to control whether or not an island’s props should be persisted upon navigation.

By default, when you add transition:persist to an island, the state is retained upon navigation, but your component will re-render with new props. This is useful, for example, when a component receives page-specific props such as the current page’s title.

You can override this behavior with transition:persist-props. Adding this directive will keep an island’s existing props (not re-render with new values) in addition to maintaining its existing state.

---

Now it works exactly the other way around. When I have transition:persist, all props are preserved, but when I use transition:persist-props, they are reset when the page changes.

[martrapp]

Hi @Fakerko there might be a miss-understanding:

For a navigation from "from" to "to",
use transition:persist + transition:name (or short transition:persist="name") on the "from" page (and the same directives on the "to" page) to get the properties reevaluated on the "to" page.
Use transition:persist + transition:name (or short transition:persist="name") + transition:persist-props on the "from" page (and transition:persist+transition:name(or shorttransition:persist="name"`) on the "to" page) to prevent reevaluation of properties on the "to" page.

Do you have a counterexample that I should have a look at?

[Fakerko]

Hi @martrapp, I made an example on Codesandbox.

The first component uses transition:persist and the second uses transition:persist-props.

Once you click on "Update values" in the components and go to the "About" page, for the component with transition:persist the component values are preserved, but for the component with transition:persist-props they are reset.

[martrapp]

Yes.

• The first component does not refresh props because the Test component does not use any. (The signature is Test = (props: any) but it does not render any props.) Test1 ... test3 is state, and as the docs say: "the state is retained upon navigation".
• The second component does not persist anything because there is no transition:persist directive.

[Fakerko]

Oh, I get it. So it's working as it should, I'm sorry.

My understanding was that transition:persist-props preserves the component values, and transition:persist preserves the component but without the values.

Thanks a lot!

[martrapp]

Let's see if we can make that more clear in the docs. ;-)

[Fakerko]

Maybe add an example at the end of transition:persist-props block, something like this:

Component will be re-rendered with new props:
<MyComponent myProp="someProp" transition:name="someName" transition:persist />

Component will keep the existing props:
<MyComponent myProp="someProp" transition:name="someName" transition:persist-props />

[martrapp]

At least we have to better point out, that transition:persist-props does not work without transition:persist!

Thank you very much for your feedback. Obviously this is complicated.