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

Add codes to warnings (and errors?) and provide default handling to onwarn function #824

Closed
Conduitry opened this issue Sep 7, 2017 · 7 comments

Comments

@Conduitry
Copy link
Member

Each warning that the compiler emits (and potentially also each error) should have a code field, intended for use by computers, and which should not change even if the actual human-friendly verbiage changes.

The default warning handler should also be passed as a second argument to a user-defined onwarn, so that users who want to specially handle (e.g., suppress) certain warnings can do so, while still having an easy way to use the normal handling for others.


This issue had its roots in the new accessibility warnings that are emitted in 1.38 - this is a class of warnings that in some cases it would be convenient to disable. There should probably be some was to distinguish a11y warnings without knowing what the codes of all the different warnings (and which will still catch all of them when new ones are added). I'm not sure whether it makes more sense to have a convention like "all a117 warnings have a code that starts with A11Y_", or to have a separate field in the warning object that indicates which general category of warning this belongs to. I'm leaning a little more toward the former. There may also be other warnings which would be nice to keep together in other groups.

@Conduitry
Copy link
Member Author

In the brief Gitter conversation leading to this issue, it was mentioned how warnings could be given a toString() method to ease the transition - However, it looks like warnings already are objects and already have a toString() method!

export interface Warning {
	loc?: { line: number; column: number; pos?: number };
	pos?: number;
	message: string;
	filename?: string;
	frame?: string;
	toString: () => string;
}

What might be slightly more icky is that there does not look to be a single point that all warnings go through. Introducing this might be a good idea, to make sure that everything onwarn will always get the same default warning handler.


For now, even though the default error handler is simply throw error;, it still might be nice to provide this as a second argument to onerror for symmetry and in case this ever gets more logic.

@fregante
Copy link

fregante commented Oct 4, 2017

This would be good.

The default warning handler should also be passed as a second argument to a user-defined onwarn, so that users who want to specially handle (e.g., suppress) certain warnings can do so, while still having an easy way to use the normal handling for others.

Instead of passing the handler, the handler should just receive whatever onwarn returns, so it can be used as a filter:

{
    onwarn: warning => {
        if (warning.code === 'A11Y_autofocus') {
          return;
        }
        warning.message += ' and stuff';
        return warning;
    }
}

@Conduitry
Copy link
Member Author

@arxpoetica (replying here to your comment in #882 as I consider this the 'main' issue)

Adding a second argument to user-specified onerror/onwarn that's the default handler has been supported for a couple months. I started poking around a bit at the time with adding codes to all of the warnings but it was boring and I stopped 😷 and it's probably not worth it at this point to find that branch because of all the refactoring that's happened since then.

It's probably worthwhile to circle back on this, as the current solution (peering at the warning messages with regexes) isn't great. I just don't have a lot of use for a better version of this personally and so I haven't been particularly motivated to work on it 😆

@arxpoetica
Copy link
Member

@Conduitry I agree, it's not priority 1. Should I close #882? Is that one still relevant?

@Conduitry
Copy link
Member Author

Codes on warnings and errors was added in 1.62

@itslenny
Copy link

itslenny commented Jan 6, 2023

This is a pretty old issue, but it is where I landed trying to disable a11y warnings. The onwarn solution above works as a global solution, but this thread does not make it clear where to add it. So, for the benefit of anyone that lands here in the future I thought I'd share a working solution to save you some searching.

To disable ALL a11y in svelte.config.js add:

const config = {

    // .....

    onwarn: (warning, handler) => {
        if (warning.code.startsWith('a11y-')) {
            return;
        }
        handler(warning);
    },

    // ......
}

You can also be less heavy-handed and just disable individual warnings by code.

@repulsio
Copy link

repulsio commented Aug 25, 2023

This is a pretty old issue, but it is where I landed trying to disable a11y warnings. The onwarn solution above works as a global solution, but this thread does not make it clear where to add it. So, for the benefit of anyone that lands here in the future I thought I'd share a working solution to save you some searching.

To disable ALL a11y in svelte.config.js add:

const config = {

    // .....

    onwarn: (warning, handler) => {
        if (warning.code.startsWith('a11y-')) {
            return;
        }
        handler(warning);
    },

    // ......
}

You can also be less heavy-handed and just disable individual warnings by code.

Does this still work for anyone? Not for me on latest SvelteKit

@Conduitry @itslenny

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