Skip to content

Diagnostic message "Stores must be declared at the top level of the component" gives invalid line number #1271

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

Closed
rgossiaux opened this issue Dec 8, 2021 · 7 comments
Labels
bug Something isn't working Fixed Fixed in master branch. Pending production release.

Comments

@rgossiaux
Copy link

Describe the bug
The diagnostic message Stores must be declared at the top level of the component comes with a line number of -1, which is invalid: https://microsoft.github.io/language-server-protocol/specification#position This breaks diagnostics in my editor.

To Reproduce
Here is a minimal file that repros this for me:

<script lang="ts">
    import { writable } from "svelte/store";

    let foo = writable("value");
    function bar() {
        let bar = foo;
        console.log($bar);
    }
</script>

Expected behavior
The line number should ideally be the line number of the underlying problem, but at the very least (if this is an upstream svelte problem) it should be validated to be non-negative, eg setting it to 0.

Screenshots
If applicable, add screenshots to help explain your problem.

System (please complete the following information):

  • OS: Mac OS 12.0
  • IDE: Neovim
  • Plugin/Package: svelte-language-server 0.14.13
@rgossiaux rgossiaux added the bug Something isn't working label Dec 8, 2021
@jasonlyu123
Copy link
Member

Did you disable sourcemap for preprocessing by any means? It seems like this diagnostic can't be mapped back to its source.

@rgossiaux
Copy link
Author

Not as far as I know but I'm happy to run any kind of check on my end. This is the only diagnostic that has a problem--everything else works correctly.

@rgossiaux
Copy link
Author

I installed VSCode to rule out anything specific to my neovim setup. I installed the svelte extension, opened up a fresh directory, made a new file, and pasted in the code in my report. While I'm not sure how to debug the underlying LSP like I am with nvim, it looks like it's broken here as well: as long as this error is in the file, the problems elsewhere in the file don't update. You can insert gibberish into the file and it will only suddenly be flagged as a problem once you remove the console.log($bar) line or otherwise fix that error.

Additionally I ran into a second diagnostic where this happens: illegal-global, with message $ is an illegal variable name. It's also sending a line of -1. If I typo $: as $; I run into this. For example:

<script lang="ts">
    $;
</script>

If I add a second line with gibberish like asldkfq;, nothing appears in VSCode. Only once I change $; to $: (for example) does something show up.

Finally it looks like this is related to the lang="ts" designation. If I remove that then everything works correctly (both in neovim and in VSCode).

@jasonlyu123
Copy link
Member

jasonlyu123 commented Dec 10, 2021

Did you have a tsconfig.json or jsconfig.json? From my debugging, it seems like the version of the svelte-preprocess we're bundling doesn't quite work when there isn't either of them. The forced sourcemap option we passed also doesn't work. We might have to consider bumping the version.

If you do have a tsconfig/jsconfig make sure you enable compilerOptions.sourceMap. This should make fix the underlying problem.

@rgossiaux
Copy link
Author

Aha, you're right. I added a tsconfig.json (extending the @tsconfig/svelte config) and these diagnostics work now. Thanks! That fixes my problem, though I still consider this a bug--if the language server runs into problems like this it should fail more gracefully than sending invalid diagnostics.

@dummdidumm
Copy link
Member

Did you have a tsconfig.json or jsconfig.json? From my debugging, it seems like the version of the svelte-preprocess we're bundling doesn't quite work when there isn't either of them. The forced sourcemap option we passed also doesn't work. We might have to consider bumping the version.

Does a version bump help us, or is this something that's still a problem inside svelte-preprocess? Or is it just a configuration problem on our end? Maybe we need to pass sourceMap: true as well?

        return {
            preprocess: sveltePreprocess({
                sourceMap: true, //<<-- add this line
                // 4.x does not have transpileOnly anymore, but if the user has version 3.x
                // in his repo, that one is loaded instead, for which we still need this.
                typescript: <any>{
                    transpileOnly: true,
                    compilerOptions: { sourceMap: true, inlineSourceMap: false }
                }
            })
        };

@jasonlyu123
Copy link
Member

Does a version bump help us

It's fixed in sveltejs/svelte-preprocess#407

though I still consider this a bug--if the language server runs into problems like this it should fail more gracefully than sending invalid diagnostic

Yeah we also should fix the line number.

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue Dec 10, 2021
Ts, svelte-preprocess, prettier
sveltejs#1271
dummdidumm added a commit that referenced this issue Dec 10, 2021
Ts, svelte-preprocess, prettier
#1271
dummdidumm pushed a commit that referenced this issue Dec 15, 2021
#1271
This would still happen if somehow the user explicitly disabled the ts sourcemap or there's other source map issues.
@jasonlyu123 jasonlyu123 added the Fixed Fixed in master branch. Pending production release. label Dec 15, 2021
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

3 participants