Revert "[feat] Suggest props destructuring if possible"

[Rich-Harris]

Reverts #6069. Unfortunately this creates buggy code, because the destructured values aren't first assigned until after the <script> contents execute.

People need to manually migrate this stuff. Offering a half measure with gotchas is worse than doing nothing IMHO.

[changeset-bot]

🦋 Changeset detected

Latest commit: 8a7483b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte-migrate	Patch

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

[Rich-Harris]

I'm not sure what the proper protocol around changesets is when reverting. Should the deleted changeset stay deleted, and should we add a new changeset?

[dummdidumm]

The old changeset stays (no deletion) and you add a new one.
Btw I didn't know if this behavior previously, guess I never ran into it 😅

[dummdidumm]

What about the alternative fix to add let { .. } = data; above the reactive statement? That one would work, and at the same time it's hopefully weird enough that people change their code eventually - but if they are in a hurry, they can use that suggestion.

[Rich-Harris]

I'm not personally keen on it — we're literally saying that it's a suggestion for how to write the code, when it's very much not. I really think it's better to encourage people to complete the migration, otherwise codebases will be left in a half-migrated state.

[geoffrich]

The migration guide also suggests destructuring data -- should we update that as well? #5774 (comment)

If you want to have quick-and-dirty migration, you can destructure the data prop using a reactive assignment - that way, you don't need to touch the rest of the file:

[Rich-Harris]

I think that's actually a good compromise — it points to the hacky solution without endorsing it. We should probably update the sample to include the initial let, to prevent people encountering the bug you found yesterday

[Rich-Harris]

(updated it)

[Rich-Harris]

gonna go ahead and merge this so we're not suggesting buggy code to people, we can always discuss unreverting it with the let change later

[kristjanmar]

Reverts #6069. Unfortunately this creates buggy code, because the destructured values aren't first assigned until after the <script> contents execute.

Is this why I've been getting frequent undefined errors when I destructure the data prop in the +page.svelte file?

For example, if I return { foo: { something: 'string' } } from +layout.server.ts and { bar: { something: 'string' } } from +page.server.ts, then I would use something like this to destructure:

export let data: PageData
$: ({ foo, bar } = data)

<div>{foo.something}</div>
<div>{bar.something}</div>

This would work most of the time, but occasionally I would get undefined errors. As if the component was trying to render before the data had been destructured properly.

If I just skip the destructring and call data.foo.something and data.bar.something inside the page component, is that the most reliable way to do it?

[dummdidumm]

Your given example should work, the html is rendered only after all reactive statements are evaluated