sveltejs / eslint-plugin-svelte

ESLint plugin for Svelte using AST
https://sveltejs.github.io/eslint-plugin-svelte/
MIT License
279 stars 30 forks source link

$page is an illegal variable name #652

Closed huntabyte closed 3 weeks ago

huntabyte commented 6 months ago

Before You File a Bug Report Please Confirm You Have Done The Following...

What version of ESLint are you using?

8.56.0

What version of eslint-plugin-svelte are you using?

2.36.0-next.3

What did you do?

Configuration ``` /** @type { import("eslint").Linter.FlatConfig } */ module.exports = { root: true, extends: [ 'eslint:recommended', 'plugin:@typescript-eslint/recommended', 'plugin:svelte/recommended' ], parser: '@typescript-eslint/parser', plugins: ['@typescript-eslint'], parserOptions: { sourceType: 'module', ecmaVersion: 2020, extraFileExtensions: ['.svelte'] }, env: { browser: true, es2017: true, node: true }, overrides: [ { files: ['*.svelte'], parser: 'svelte-eslint-parser', parserOptions: { parser: '@typescript-eslint/parser' } } ] }; ```
  1. pnpm create svelte@latest
  2. Choose Skeleton Project, Typescript, Eslint, Svelte 5
  3. install dependencies
  4. Realize eslint-plugin-svelte@next isn't installed, so pnpm i -D eslint-plugin-svelte@next
<!-- src/routes/+page.svelte -->
<script lang="ts">
    import { page } from "$app/stores"
    console.log($page.data) // $page is an illegal variable name.
</script>

What did you expect to happen?

$page to be a legal variable name

What actually happened?

$page is an illegal variable name. To reference a global variable called $page, use globalThis.$page(illegal-global)eslint[svelte/valid-compile](https://sveltejs.github.io/eslint-plugin-svelte/rules/valid-compile/)
let $page: Page<Record<string, string>, string | null>

4:14 error $page is an illegal variable name. To reference a global variable called $page, use globalThis.$page(illegal-global) svelte/valid-compile

Link to GitHub Repo with Minimal Reproducible Example

https://github.com/huntabyte/s5-illegal-variable-repro

Additional comments

I noticed this only occurs when using Typescript. If I go through the create svelte flow and choose "Javascript with JSDoc" everything is good.

lyssieth commented 6 months ago

This also happens for a custom store (in my case secret or $secret) as well.

Also Svelte 5, Eslint, TS.

christophsturm commented 4 months ago

is there a workaround? btw, where is the source of the next plugin, in case i want to work on a PR

RyousukeUchino commented 3 months ago

@christophsturm @huntabyte

I'm not entirely sure if this is correct, but I was able to find a workaround using the following method:

<script lang="ts">
  import { page } from '$app/stores';
  import { get } from 'svelte/store';

  const currentPagePath = get(page).url.pathname;
  const currentPageData = get(page).data
</script>
{#if currentPagePath === '/store'}
  hoge fuga
{/if}
unlocomqx commented 3 months ago

Is it possible to disable this error?

jill64 commented 3 months ago

As you may already know, the @RyousukeUchino proposal may suppress warnings, but is not recommended in hot code paths.

https://svelte.dev/docs/svelte-store#get

This works by creating a subscription, reading the value, then unsubscribing. It's therefore not recommended in hot code paths.

jill64 commented 3 months ago

@unlocomqx

The following can be used as a workaround

Disable in one line

<script>
  import { page } from '$app/stores';

  // eslint-disable-next-line svelte/valid-compile
  const currentPagePath = $page.url.pathname;
</script>

Disable in project

.eslintrc.cjs

module.exports = {
  // ...
  rules: {
    "svelte/valid-compile": "off"
  }
}

Check here for additional details https://eslint.org/docs/latest/use/configure/rules

ryanatkn commented 3 months ago

The get proposal is also unsuitable as a general workaround because it's non-reactive. It subscribes and immediately unsubscribes, whereas $ subscribes and doesn't unsubscribe until the component is destroyed.

Conduitry commented 2 months ago

This plugin appears to be incorrectly transforming the <script> tag (perhaps via TypeScript?) before passing it to the compiler.

Something like

<script>
    import { page } from '$app/stores';
</script>

is getting transformed into

<script>import '$app/stores';
//# sourceMappingURL=module.js.map
</script>

presumably because it's only operating on the <script> tag and it thinks page is unused and it's safe to turn that into a bare import '$page/stores'.

Putting a bare reference to page; in the <script> tag seems to be a workaround for the time being.

<script>
    import { page } from '$app/stores';
    page;
</script>

gets transformed into

<script>import { page } from '$app/stores';
page;
//# sourceMappingURL=module.js.map
</script>

but the real solution is going to be fixing this transformation that the plugin does to the script tag before passing it to the Svelte compiler. If this is related to TypeScript, I think the solution is going to be to just stop running the <script> tag through TypeScript in Svelte 5, because that should be already handled by the Svelte compiler.

unlocomqx commented 2 months ago

I tried adding page as a global in eslint config no luck

not a big deal though

dummdidumm commented 2 months ago

From @Conduitry's investigation my shot in the dark is that ts.transpileModule (or whatever the low level transpiration API is called) is called with the wrong config, where imports are not left alone and instead "treeshaken". verbatimModuleSyntax was introduced for this reason in TS and is the recommended default since TS 5.0, and leaves imports as is.

Conduitry commented 2 months ago

@dummdidumm In Svelte 5, is there a reason to transpile the <script> tag at all before passing it to the Svelte compiler, though?

rChaoz commented 2 months ago

I don't think so, the Svelte 5 compiler natively supports Typescript (now even in the template, not just the script tag).

niemyjski commented 1 month ago

I'm running into this as well.

ptrxyz commented 1 month ago

I tried setting verbatimModuleSyntax to true, but it doesn't seem to have an effect. So yes, probably it is using a wrong configuration. :/

Alia5 commented 1 month ago

I don't think so, the Svelte 5 compiler natively supports Typescript (now even in the template, not just the script tag).

It doesn't fully support all TS features, decorators and enums among them, so there might still be a need to preproccess the files beforehand.

KieranP commented 1 month ago

Went about converting my app from Svelte 4 -> 5 today, update process was fairly smooth, but the eslint errors in this ticket are showing up. Would be good to see a proper fix for this that doesn't involve disabling eslint checks everywhere