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

Unused export property warning is on the wrong line when using the TypeScript pre-processor. #489

Closed
brunnerh opened this issue Aug 26, 2020 · 22 comments
Labels
bug Something isn't working

Comments

@brunnerh
Copy link
Member

Describe the bug
export let statements of an unused property cause a warning. The warning is displayed on the line above the actual property for me.

To Reproduce

Set the script language to typescript (lang="ts" or type="text/typescript").
Declare some export property, do not use it anywhere.

Expected behavior

The warning is on the property not being used.

Screenshots

image

If the property is in the first line of the script:

image

System (please complete the following information):

  • OS: Windows 10, 64bit
  • IDE: VSCode
  • Plugin/Package: Svelte for VSCode
@brunnerh brunnerh added the bug Something isn't working label Aug 26, 2020
@jasonlyu123
Copy link
Member

Try update your @tsconfig/svelte to lastest

@brunnerh
Copy link
Member Author

I am not using that.
Added it temporarily, did nothing.

@jasonlyu123
Copy link
Member

Oh I thought you were using the official template. The package is a base config for svelte. You can update the extends of your tsconfig.json to match this.
https://github.com/sveltejs/template/blob/8194bf8f6452f8117195bf304ac99b15d329fbb3/scripts/setupTypeScript.js#L87

@jasonlyu123
Copy link
Member

jasonlyu123 commented Aug 27, 2020

Duplicate of #333

@jasonlyu123 jasonlyu123 marked this as a duplicate of #375 Aug 27, 2020
@jasonlyu123 jasonlyu123 marked this as a duplicate of #333 Aug 27, 2020
@jasonlyu123 jasonlyu123 marked this as not a duplicate of #375 Aug 27, 2020
@brunnerh
Copy link
Member Author

@jasonlyu123 As i said, i added the base config temporarily but nothing changed. I still get the errors on the wrong line.

@jasonlyu123
Copy link
Member

Then can you try run tsc to compile a ts file in your workspace and see if the output file has sourcemap?

@jasonlyu123
Copy link
Member

jasonlyu123 commented Aug 27, 2020

The problem is with sourcemap. We use the svelte compiler to get svelte diagnostics. if you have a svelte.config.js and specify your preprocessor in it we would use that to preprocess, other than that we'll use the official svelte-preprocess. And if the preprocessor doesn't return a sourcemap the warning would be in a wrong position.

Also, we don't monitor change in the tsconfig.json so make sure you restart the language server after any changes.

@brunnerh
Copy link
Member Author

It does not always generate source maps. If i call tsc on a single file it apparently does not load the tsconfig; if i use tsc -p . or tsc --build tsconfig.json it will output the source map as expected.

@jasonlyu123 jasonlyu123 added question Further information is requested and removed bug Something isn't working labels Aug 28, 2020
@jasonlyu123 jasonlyu123 reopened this Aug 28, 2020
@jasonlyu123
Copy link
Member

Can I take a look at your svelte.config.js if you have it, also which preprocessor do you use?

@brunnerh
Copy link
Member Author

I use the default processor, which should be svelte-preprocess in auto mode. I made a minimal example repository which has the errors on the input component.

I tried creating a svelte.config.js, explicitly setting the pre-processor in auto mode, did not matter. Explicitly using only the typescript one with a path to the tsconfig.json did not do anything either.

@jasonlyu123
Copy link
Member

Ummm... this is unexpected, it would only work when I also set sourcemap to true in tsconfig.json

@brunnerh
Copy link
Member Author

Then the extends does not quite work. Well, i do not mind setting that explicitly for now.
Thanks

@jasonlyu123 jasonlyu123 added the bug Something isn't working label Aug 28, 2020
@jasonlyu123
Copy link
Member

jasonlyu123 commented Aug 28, 2020

Yeah, I have a quick look at the source file of the svelte-preprocess. It looks like it doesn't parse the extends.
Thanks for the issue!

@dummdidumm
Copy link
Member

dummdidumm commented Aug 28, 2020

I opened this issue over at svelte-preprocess, because according to the docs, in auto-preprocessing mode, source maps should be generated. Update: The docs were wrong, so you need to set that to true yourself.

@brunnerh if you remove "sourceMaps": true and the base config reference from your tsconfig.json and instead do sveltePreprocess({sourceMap: true}) in your svelte.config.js, does it show the warnings at the correct line then?

dummdidumm pushed a commit that referenced this issue Aug 28, 2020
…495)

This will add source maps for people who do not have a svelte.config.js inside their workspace.
#489
@brunnerh
Copy link
Member Author

@brunnerh if you remove "sourceMaps": true and the base config reference from your tsconfig.json and instead do sveltePreprocess({sourceMap: true}) in your svelte.config.js, does it show the warnings at the correct line then?

No, still off.

@dummdidumm
Copy link
Member

So it only works if you add "sourceMaps": true to the tsconfig.json?

@brunnerh
Copy link
Member Author

Yes

@dummdidumm
Copy link
Member

Closing as these are problems related to svelte-preprocess. The "source map is not applied when I tell preprocess through its sourceMap-option"-problem is fixed in the latest version. The "extends-feature of tsconfig.json not supported" is tracked in sveltejs/svelte-preprocess#300 .

@stefanosandes
Copy link

Hello Guys!

I don't know if this issue is being tracked elsewhere, but I believe this problem is sensitive to changes in svelte.config.js.

I am using MeltUI, so here are the results:
Group 2074

Just variations 1 and 2 show the warning in the correct position.

The variations are:

import sequence from 'svelte-sequential-preprocessor';
import adapter from '@sveltejs/adapter-vercel';
import { vitePreprocess } from '@sveltejs/kit/vite';

1 - OK: preprocess:vitePreprocess(),
2 - OK: preprocess:[vitePreprocess()],
3 - WRONG: preprocess:[vitePreprocess(), preprocessMeltUI()],
4 - WRONG: preprocess:[preprocessMeltUI(), vitePreprocess()],
5 - WRONG: preprocess:sequence([vitePreprocess()],

@martinjo
Copy link

Hello Guys!

I don't know if this issue is being tracked elsewhere, but I believe this problem is sensitive to changes in svelte.config.js.

I am using MeltUI, so here are the results: Group 2074

Just variations 1 and 2 show the warning in the correct position.

The variations are:

import sequence from 'svelte-sequential-preprocessor';
import adapter from '@sveltejs/adapter-vercel';
import { vitePreprocess } from '@sveltejs/kit/vite';

1 - OK: preprocess:vitePreprocess(), 2 - OK: preprocess:[vitePreprocess()], 3 - WRONG: preprocess:[vitePreprocess(), preprocessMeltUI()], 4 - WRONG: preprocess:[preprocessMeltUI(), vitePreprocess()], 5 - WRONG: preprocess:sequence([vitePreprocess()],

Did you solve this? I’m running into the same problem when trying to use MeltUI in my SvelteKit project.

@stefanosandes
Copy link

Hello Guys!
I don't know if this issue is being tracked elsewhere, but I believe this problem is sensitive to changes in svelte.config.js.
I am using MeltUI, so here are the results: Group 2074
Just variations 1 and 2 show the warning in the correct position.
The variations are:

import sequence from 'svelte-sequential-preprocessor';
import adapter from '@sveltejs/adapter-vercel';
import { vitePreprocess } from '@sveltejs/kit/vite';

1 - OK: preprocess:vitePreprocess(), 2 - OK: preprocess:[vitePreprocess()], 3 - WRONG: preprocess:[vitePreprocess(), preprocessMeltUI()], 4 - WRONG: preprocess:[preprocessMeltUI(), vitePreprocess()], 5 - WRONG: preprocess:sequence([vitePreprocess()],

Did you solve this? I’m running into the same problem when trying to use MeltUI in my SvelteKit project.

Not a solution, but a workaround. I removed the MeltUI preprocessor and am now using Melt components in a destructuring style.

@martinjo
Copy link

Ok, thanks! Seems like the most reasonable thing to do. I've already spent way too much time looking for a solution to this now 🤦‍♂️

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants