Fix: prevent island from re-rendering when using transition:persist

[azhirov]

This PR addresses the issue described in #11854.

Issue: In the swapBodyElement function, the props attribute of the custom element is being updated even when the new and old values are identical. This unnecessary update triggers rehydration, which can lead to unexpected behavior during transitions. swap-functions.ts#L82

When navigating to a different page containing the same component (island), the following method is triggered on the custom element astro-island:

attributeChangedCallback() {
    this.hydrate();
}

(Source: astro-island.ts#L204)

Changes

I introduced a check to compare the props attribute values between el and newEl. The update now only occurs if the values differ.

However, since these strings represent serialized objects, there is a potential concern about whether a simple string comparison is sufficient, considering that the property order within the serialized objects might vary.

Testing

I tested the changes by navigating between pages that contain the same component (island) to ensure that the props attribute is not unnecessarily updated, preventing redundant rehydration. The component state now persists correctly across transitions without triggering unnecessary rehydration.

The testing was conducted using the code available at https://stackblitz.com/edit/github-jhuzfh-jxxerz. This environment allowed me to reproduce the issue and verify the effectiveness of the fix.

No new tests were added since this change addresses a specific issue with existing behavior, and the existing test suite sufficiently covers the transition and hydration logic. However, I verified that all existing tests pass, ensuring that there are no regressions caused by this fix.

Docs

This change does not affect user-facing behavior or API, so no updates to the documentation are needed.

[changeset-bot]

🦋 Changeset detected

Latest commit: 42cd338

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

[martrapp]

Hi @azhirov 👋🏼, thank you very much for providing a fix for #11854!

That looks good to me: If I understand your solution correctly, we don't update the props if they haven't changed.

From the original issue:

The usecase where a component state is persisted and still react to props change is broken for vue, svelte and solid

Could you please add an e2e test using a Vue component (similar to the existing island-two.astro/island-one.astro pair) that shows that updating props now works without errors for persisted components when the props do change?

[johannesspohr]

I opened a PR (#11946) to fix the Vue issue, that should also work in case of changed props.

[matthewp]

lgtm, aside from needing an E2E like @martrapp suggested.

[martrapp]

Hi @azhirov and @johannesspohr: How about merging your two PRs? #11915 looks as if it would work with other frameworks beyond Vue, and #11946 does have the e2e test!

[johannesspohr]

Hi @azhirov and @johannesspohr: How about merging your two PRs? #11915 looks as if it would work with other frameworks beyond Vue, and #11946 does have the e2e test!

If I'm not mistaken, this PR only handles situations where the props are not changed, but if they change, it's still broken (at least I couldn't find any code to update the props of islands other than React)
I implemented said update logic for Vue in my PR to the best of my knowledge, but it still needs approval. The other integrations (Svelte, Solid, Alpine) would also require a similar implementation. If it is working for all FE frameworks, the change from this PR would probably not be needed anymore.