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

Svelte 5: svelte-ignore not working #11414

Closed
rChaoz opened this issue May 1, 2024 · 8 comments
Closed

Svelte 5: svelte-ignore not working #11414

rChaoz opened this issue May 1, 2024 · 8 comments
Milestone

Comments

@rChaoz
Copy link
Contributor

rChaoz commented May 1, 2024

Describe the bug

It seems like svelte-ignore comments are not working since I upgraded to Svelte 5 - svelte-check and my IDE are spewing out errors/warnings where there were none before. This may seem like something small but currently it's blocking my CI and I'm not sure how to fix it, this is why it put this severity.

Reproduction

<!-- svelte-ignore a11y-no-static-element-interactions -->
<div on:mousedown={...}></div>

Logs

Warn: `<div>` with a mousedown or mouseup handler must have an ARIA role (svelte)
[...]

System Info

System:
    OS: Windows 10 10.0.19045
    CPU: (20) x64 12th Gen Intel(R) Core(TM) i7-12700KF
    Memory: 13.10 GB / 31.86 GB
  Binaries:
    Node: 20.8.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 4.1.1 - C:\Program Files\nodejs\yarn.CMD
    npm: 10.2.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0)
    Internet Explorer: 11.0.19041.1566

Severity

blocking an upgrade

@7nik
Copy link

7nik commented May 1, 2024

Error and warning codes were switched to snake style: a11y_no_static_element_interactions, and some were renamed. It's shortly listed in the breaking changes.

@Conduitry
Copy link
Member

We should either update the "Various error and warning codes have been renamed slightly." note in the docs to say that all error and warning codes have been changed from kebab case to snake case, or we should have svelte-ignore normalize kebab case to snake case when it's comparing error codes.

@7nik
Copy link

7nik commented May 1, 2024

For DX, it would be best to map the old codes to the new ones but emit a warning about using old codes.

@huntabyte
Copy link
Member

Alternatively, a migration script could be added, similar to the self-closing tags, to take care of it.

@dummdidumm dummdidumm added this to the 5.0 milestone May 1, 2024
@rChaoz
Copy link
Contributor Author

rChaoz commented May 1, 2024

I tried updating to the new codes, and it's not working for 2 main reasons:

  • IDE support - my IDE (JetBrains) gives me quick actions to add the svelte-ignore tags with the old codes and warnings when new ones are used
  • eslint-plugin-svelte gives me 2 errors when I switch to the new style codes:
    • on the comment itself - ESLint: svelte-ignore comment is used, but not warned(svelte/no-unused-svelte-ignore)
    • on the autofocus tag - ESLint: A11y: Avoid using autofocus(a11y-autofocus)(svelte/valid-compile)

Personally, I think that this breaks a lot of things, possibly more than initially thought, and I don't really see a valid reason for this change? Especially when every other tool I can think of uses kebab-case for its comments. Could this potentially be reverted/modified to accept using the old style comments without warnings?

@rChaoz
Copy link
Contributor Author

rChaoz commented May 2, 2024

Update: I updated ESLint & its svelte plugin to latest version, they now support the snake_case error IDs. However, it seems like it's not very consistent. It works for almost all cases, but the following still fails:

<!-- svelte-ignore a11y_autofocus -->
<input type="text" autofocus style="display: none" />
$ svelte-check
Warn: Avoid using autofocus (svelte)
        <!-- svelte-ignore a11y_autofocus -->
        <input type="text" autofocus style="display: none" />

Others, like a11y_no_static_element_interactions work correctly.

@7nik
Copy link

7nik commented May 2, 2024

a11y_autofocus seems to be renamed to a11y_misplaced_role.

dummdidumm added a commit that referenced this issue May 14, 2024
Related to #11414

---------

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
@dummdidumm
Copy link
Member

Closed by #11549

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants