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

docs: clarify svelte:component migration, avoids common gotcha #13835

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rgon
Copy link
Contributor

@rgon rgon commented Oct 23, 2024

Clarify docs to specify that, while svelte:component is no longer needed and deprecated, capitalizing the component variable name is required to avoid mismatching types.

Avoids users doing the same gotcha that I stumbled upon, it will be a common oversight when refactoring existing codebases: #13795

As evidenced in these docs themselves (corrected):

...
	let component: Component<{ foo: string }> = $state(
		Math.random() ? ComponentA : ComponentB
	);
</script>

<svelte:component this={component} foo="bar" />

Copy link

changeset-bot bot commented Oct 23, 2024

⚠️ No Changeset found

Latest commit: 0159eda

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@@ -699,9 +714,9 @@ In Svelte 4, doing the following triggered reactivity:

This is because the Svelte compiler treated the assignment to `foo.value` as an instruction to update anything that referenced `foo`. In Svelte 5, reactivity is determined at runtime rather than compile time, so you should define `value` as a reactive `$state` field on the `Foo` class. Wrapping `new Foo()` with `$state(...)` will have no effect — only vanilla objects and arrays are made deeply reactive.

### `<svelte:component>` is no longer necessary
### `<svelte:component>` is no longer necessary {#svelte-component-no-longer-necessary}
Copy link
Member

Choose a reason for hiding this comment

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

what is the {#svelte-component-no-longer-necessary} for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a heading ID so that it can be referenced in the section 'Component typing changes'

Or using the [new syntax](#svelte-component-no-longer-necessary):

I added it since the example block of code used the old dynamic component syntax (and I don't believe this should be changed), so while sequentially reading the migration docs, a bit of a reminder that there's new syntax, where to find it and an example of old vs new is nice.

Copy link
Member

Choose a reason for hiding this comment

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

All headings are automatically made linkable. You don't need to do anything extra

Suggested change
### `<svelte:component>` is no longer necessary {#svelte-component-no-longer-necessary}
### `<svelte:component>` is no longer necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair, I customized it since the autogenerated one depends on its parent section as well, and thus seems quite fragile: #Breaking-changes-in-runes-mode-svelte:component-is-no-longer-necessary. For example, it will break if this section is moved out of the parent heading.

However, now that I realize, it shouldn't be in the Breaking-changes-in-runes-mode section since it's not a breaking change, it just throws a deprecation warning REPL.

Modifying this PR now to move the section outside Breaking changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rebased to main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants