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

Props removed when saving #2594

Open
frederichoule opened this issue Nov 18, 2024 · 12 comments
Open

Props removed when saving #2594

frederichoule opened this issue Nov 18, 2024 · 12 comments
Labels
bug Something isn't working Fixed Fixed in master branch. Pending production release.

Comments

@frederichoule
Copy link

Describe the bug

When VSCode adds an import automatically, it adds it under the let { data }: { data: PageData } = $props();, which then gets removed when saving, probably due to auto-formatting.

Reproduction

  1. Create a +page.svelte file
  2. Write this: (Normally you wouldn't add the "error" import yourself, it would be added by VSCode when adding that function in your code somewhere in the file.)
import type { PageData } from './$types';
let { data }: { data: PageData } = $props();
import { error } from '@sveltejs/kit';
  1. Save (or format file)
  2. This will result in the let {data} being deleted.
import { error } from '@sveltejs/kit';
import type { PageData } from './$types';

Expected behaviour

Either the import { error } should be added before the let { data } initially, or the formatting tool places it at the correct place and do not delete the let { data }.

System Info

  • OS: macOS 15.1
  • IDE: VSCode 1.95.3

Which package is the issue about?

Svelte for VS Code extension

Additional Information, eg. Screenshots

CleanShot 2024-11-18 at 07 32 57@2x
CleanShot 2024-11-18 at 07 33 06@2x

@frederichoule frederichoule added the bug Something isn't working label Nov 18, 2024
@jasonlyu123
Copy link
Member

You probably have the editor.codeActionsOnSave config. What is the config? if it has source.organizeImports then this is a duplicate of #1590. It's a limitation of how the code is transformed under the hood and we haven't found a way to workaround it yet.

I can't reproduce auto-import adding import to after props. Can you provide a code snippet and instructions for reproducing it?

@frederichoule
Copy link
Author

Yeah it seems like a duplicate of #1590

Here is a video that shows when an import is added after the let { data }.

CleanShot.2024-11-19.at.10.10.26.mp4

@frederichoule
Copy link
Author

frederichoule commented Nov 19, 2024

And I forgot to confirm:

"editor.codeActionsOnSave": {
    "source.organizeImports": "always"
},

@dummdidumm dummdidumm added the Fixed Fixed in master branch. Pending production release. label Nov 20, 2024
dummdidumm added a commit that referenced this issue Nov 20, 2024
#2594

As a drive-by, I also ensures that the dts generation knows about the hoisted interfaces so it does not transform them into types anymore
@dummdidumm
Copy link
Member

Fixed the auto import, for the more general problem I defer to #1590

dummdidumm added a commit that referenced this issue Nov 20, 2024
#2594

As a drive-by, I also ensures that the dts generation knows about the hoisted interfaces so it does not transform them into types anymore
@mcullifer
Copy link

mcullifer commented Nov 22, 2024

Can confirm this is happening on 109.3.0.
From my testing, it isn't just let { data }: {data: PageData} = $props() being sandwiched between import statements, it was happening for any top level variable assignments in a svelte component const, let, etc.

Example

<script lang="ts">
    import X from '...';
    import Y from '...';
    import Z from '...';

    let { data }: { data: PageData } = $props();
</script>

Turns into

<script lang="ts">
    import X from '...';
    import Y from '...';
    import Z from '...';
{ data: PageData } = $props();
</script>

If I have any other top level variable declarations and put them before the $props() assignment then those get removed up to the type of the $props().

<script lang="ts">
    import X from '...';
    import Y from '...';
    import Z from '...';

    let test = $state('');
    let test2 = $state(true);
    const test3 = "test3"; 
    let { data }: { data: PageData } = $props();
</script>

Turns into

<script lang="ts">
    import X from '...';
    import Y from '...';
    import Z from '...';
{ data: PageData } = $props();
</script>

Interestingly

The bug doesn't happen at all when using a named type for props instead of doing it inline

<script lang="ts">
    import X from '...';
    import Y from '...';
    import Z from '...';

    type MyProps = {
        data: PageData;
    }
    let test = $state('');
    let test2 = $state(true);
    const test3 = "test3"; 
    let { data }: MyProps = $props();
</script>

It could be related to one of my imports having side effects? Like maybe something declares a variable at the top level, but I haven't pinned that down yet as most of my imports are from 3rd party npm modules. For now though I'm just setting "source.organizeImports": "never".

@frederichoule
Copy link
Author

frederichoule commented Nov 22, 2024

109.3.0 seems to introduce a new bug.

When organize imports runs, it rewrites

let { data }: { data: PageData } = $props();

to

{ data: PageData } = $props();

Which makes 109.3.0 unusable for me. Just reverted to 109.2.4.

Edit: Just saw @mcullifer comment. I have the same exact issue.

@dummdidumm
Copy link
Member

Should be fixed in the latest release

@lts20050703
Copy link

lts20050703 commented Nov 22, 2024

@dummdidumm I can still reproduce mcullifer's issue in 109.3.2

@dummdidumm
Copy link
Member

Please provide a complete code snippet that reproduces this

@dummdidumm dummdidumm reopened this Nov 22, 2024
@frederichoule
Copy link
Author

@dummdidumm I can still reproduce mcullifer's issue in 109.3.2

The issues seems to be fixed for me with 109.3.2.

@lts20050703
Copy link

@dummdidumm

<script lang="ts">
	import {} from "svelte"
	let {}: {} = $props()
</script>

102.3.2 formats to

<script lang="ts">
	import { } from "svelte"
 {} = $props()
</script>

.vscode/settings.json

{
	"editor.codeActionsOnSave": { "source.organizeImports": "always" }
}

@frederichoule
Copy link
Author

Also:

	let { data }: { data: PageData } = $props();

	import {} from "svelte"

becomes:

	let { data }:import { } from "svelte";
import {} from "svelte"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Fixed Fixed in master branch. Pending production release.
Projects
None yet
Development

No branches or pull requests

5 participants