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

Explain that props are updated on navigation #6915

Merged
merged 8 commits into from
Mar 8, 2024
Merged

Conversation

matthewp
Copy link
Contributor

Description (required)

With withastro/astro#10136 transition:persist is changing to update props upon navigation when used on an island. This explains that props will trigger a re-render.

Related issues & labels (optional)

For Astro version: 4.5. See astro PR #10136.

Copy link

vercel bot commented Feb 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Mar 8, 2024 9:46pm
1 Ignored Deployment
Name Status Preview Updated (UTC)
docs-i18n ⬜️ Ignored (Inspect) Mar 8, 2024 9:46pm

@sarah11918 sarah11918 added this to the 4.5.0 milestone Feb 16, 2024
@sarah11918 sarah11918 added merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) minor-release For the next minor release; in the milestone, "merge queue" when approved by Sarah! labels Feb 16, 2024
@sarah11918
Copy link
Member

sarah11918 commented Mar 8, 2024

@matthewp - Is this example (in the same section, but just above what you added) still accurate? How are we making clear the distinction between a component's props vs its state?

This change now updates island props upon navigation, so would this counter get set back to 5 by default?

image

Can you make sure reading this entire section on state, that everything in it still makes sense?

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @matthewp !

I took some of the description from the language of the changeset in the implementation PR to provide a use-case, and to include the version it was introduced.

Please also read the entire section now that the default props behaviour has changed, and make sure the existing description here is still accurate, or whether mention of the new default behaviour might need to be corrected earlier, too!

src/content/docs/en/guides/view-transitions.mdx Outdated Show resolved Hide resolved
@matthewp
Copy link
Contributor Author

matthewp commented Mar 8, 2024

@sarah11918 It might be a good idea to rename count to initialCount or something, would that make it more clear? In a case like this you are really providing the initial value to the component. The component then keeps the "real" count in its state. That state will not be affected by navigation.

@matthewp
Copy link
Contributor Author

matthewp commented Mar 8, 2024

@sarah11918 reworded things just a little bit, let me know what you think?

@sarah11918 sarah11918 changed the base branch from main to 4.5.0 March 8, 2024 14:11
@sarah11918 sarah11918 added the Merge Queue Approved and ready to be merged (wait for feature release if also labelled M-O-R)! label Mar 8, 2024
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved!

@sarah11918 sarah11918 merged commit 07b8622 into 4.5.0 Mar 8, 2024
2 of 3 checks passed
@sarah11918 sarah11918 deleted the vt-preserve-props branch March 8, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge Queue Approved and ready to be merged (wait for feature release if also labelled M-O-R)! merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) minor-release For the next minor release; in the milestone, "merge queue" when approved by Sarah!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants