chore: fix $props.id tests

[paoloricciuti]

Currently tests for $props.id are a bit brittle because the id will be shared between all tests...this means that if we add a new test that uses $props.id before the current test the old test will fail for apparently no reason. This can be very confusing so we definitely need to fix this.

I changed the test to make sure that all the ids are different and in hydrate mode i've also cut the numbers out and checked that the first ones starts with s meaning they are the same as the server. I don't know if this is the right approach but it's the only one i could think of to actually test that the values comes from the server.

cc @adiguba maybe is worth for you to take a look at this.

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.
• If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

⚠️ No Changeset found

Latest commit: 7ffc25e

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

[github-actions]

Playground

pnpm add https://pkg.pr.new/svelte@15294

[Rich-Harris]

What if we instead exposed some mechanism for resetting the counter at the start of each test, similar to the (badly named) raf mechanism for controlling requestAnimationFrame-related stuff? Test files can access Svelte internals, so it doesn't even need to be exposed publicly

[paoloricciuti]

What if we instead exposed some mechanism for resetting the counter at the start of each test, similar to the (badly named) raf mechanism for controlling requestAnimationFrame-related stuff? Test files can access Svelte internals, so it doesn't even need to be exposed publicly

Mh...yeah that's a good idea. Will do in a min

[adiguba]

Currently tests for $props.id are a bit brittle because the id will be shared between all tests...

I didn't known how test works exactly, but in theory the id should be reset on each call to render()
I don't have time now, but I will look at that this week-end...

I don't know if this is the right approach but it's the only one i could think of to actually test that the values comes from the server.

Yes server-side id starts with 's', and client-side starts with 'c'...

[paoloricciuti]

What if we instead exposed some mechanism for resetting the counter at the start of each test, similar to the (badly named) raf mechanism for controlling requestAnimationFrame-related stuff? Test files can access Svelte internals, so it doesn't even need to be exposed publicly

Seems to work fine...also on the server there was not such problem because it reset on every render so i've only added it before the client mounting/hydrating.

I've only added it to runtime runes, do you think we should also add this to other test suites?

[adiguba]

See that you found the problem. 👍
I didn't understand that the problem was in "client mode"...

Also, by rereading the code I saw that I had added an option.uid in render() in order to pass a custom id generator :

svelte/packages/svelte/src/internal/server/index.js

Lines 100 to 104 in 32b78c9

	* @param {{ props?: Omit<Props, '$$slots' | '$$events'>; context?: Map<any, any>, uid?: () => string }} [options]
	* @returns {RenderOutput}
	*/
	export function render(component, options = {}) {
	const uid = options.uid ?? props_id_generator();

I totally forgot to mention it on the PR, and now I think that it is useless and should not be there.
Should I open a PR to remove that ?

[paoloricciuti]

I totally forgot to mention it on the PR, and now I think that it is useless and should not be there.
Should I open a PR to remove that ?

Uh I was wondering about that too...yeah I think it's fine to remove it if you can open a pr