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

disable validation with magic comments #3351

Merged
merged 2 commits into from
Aug 4, 2019
Merged

Conversation

Rich-Harris
Copy link
Member

This is an alternative to #1861 with more specificity.

The idea is that you can add a comment like this...

<!-- svelte-ignore a11y-missing-attribute -->

...and it will ignore any a11y-missing-attribute warnings on the next non-text, non-comment node (and any of its children). That includes {#if ...} etc and components.

So in a case like this...

<img src="whatever.jpg">

<!-- svelte-ignore a11y-missing-attribute -->
<div>
  <marquee>
    <img src="whatever.jpg">
  </marquee>

  <img src="whatever.jpg">
</div>

...we'd get an accessibility warning for the first <img> but not the second or third, and we'd get a separate warning for the <marquee>.

Multiple codes can be passed at once:

<!-- svelte-ignore a11y-missing-attribute a11y-distracting-elements -->
<div>
  <marquee>
    <img src="whatever.jpg">
  </marquee>
</div>

They can be separated by newlines:

<!-- svelte-ignore a11y-missing-attribute
                   a11y-distracting-elements -->
<div>
  <marquee>
    <img src="whatever.jpg">
  </marquee>
</div>

They can be stacked:

<!-- svelte-ignore a11y-missing-attribute -->
<!-- svelte-ignore a11y-distracting-elements -->
<div>
  <marquee>
    <img src="whatever.jpg">
  </marquee>
</div>

And they accumulate (i.e. earlier ignores are not cancelled out by newer ignores):

<!-- svelte-ignore a11y-distracting-elements -->
<div>
  <!-- svelte-ignore a11y-missing-attribute -->
  <marquee>
    <img src="whatever.jpg">
  </marquee>
</div>

Thoughts?

@Conduitry
Copy link
Member

I think this seems reasonable enough, and is a more user-friendly thing to do than writing a custom onwarn handler. I'm not sure, though, how this should be documented. There isn't anything else like this in the language really.


Do we want to worry about a way to disable warnings that occur in <script> or <style> blocks? Or would these simply have the warning disabled for the whole thing by putting the comment before the opening tag?

@Rich-Harris
Copy link
Member Author

Yeah, not sure about documentation. Needs some thought.

Looking through the code, there are only a handful of warnings that can arise from within <script>:

  • 'foo' is not defined if you reference $foo
  • $: has no effect in a module script
  • $: has no effect outside of the top-level

In <style>:

  • Unused CSS selector

Elsewhere:

  • No custom element 'tag' option was specified. To automatically register a custom element, specify a name with a hyphen in it, e.g. <svelte:options tag="my-thing"/>. To hide this warning, use <svelte:options tag={null}/>

I'm not sure if there's any real value in being able to disable those warnings — with the possible exception of the $foo thing, I can't think of any reason you'd want to disable those warnings instead of acting on them.

Separately, I wondered about having a central registry of warnings, since they're a bit scattered around at the moment. That way, we could check that someone wasn't trying to ignore a non-existent warning.

@Rich-Harris
Copy link
Member Author

I reckon this'll do:

Screen Shot 2019-08-04 at 12 15 55

@matryer
Copy link

matryer commented Jan 31, 2020

@Rich-Harris How do we find out the identifier for the warnings (e.g. a11y-autofocus) from the warning? Could the warnings (when logged out) include them?

Specifically, how do I find out which one to use for this warning:

(!) Plugin svelte: A11y: <a> element should have child content

@wrgoldstein
Copy link

@matryer they're in the svelte compiler: https://github.com/sveltejs/svelte/blob/master/src/compiler/compile/nodes/Element.ts#L668 (search for the warning text)

@opensas
Copy link
Contributor

opensas commented May 9, 2020

vscode correctly reporrts the identifier for the warning:

image

where as the message from the console is just:

4: </script>
/home/sas/devel/apps/svelte/freecodecamp/covid19/src/node_modules/components/Nav.svelte
A11y: <a> element should have an href attribute

Ideally the warning identifier could appear on the console message too

@ghost
Copy link

ghost commented Jan 5, 2022

@Rich-Harris Hi. Can you answer my question, please? Why do you add a feature and later add a feature to disable it? Is it a smart thing to do?

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

Successfully merging this pull request may close these issues.

None yet

5 participants