sveltejs / eslint-plugin-svelte3

An ESLint plugin for Svelte v3 components.
MIT License
373 stars 43 forks source link

Give ignore-warnings functions full warning object #11

Closed tivac closed 5 years ago

tivac commented 5 years ago

https://github.com/sveltejs/eslint-plugin-svelte3/blob/2ac4f8d2f0a44361b8e8910a4a60818750ce19c1/index.js#L162

Currently it looks like a function defined to handle warnings only gets passed the code value, though the svelte compiler actually returns multiple properties for a warning. I understand not passing the location pieces through but I'd at least like to be able to access the message property.

if #10 gets fixed I personally stop caring about this, because I'm trying to add a programmatic warning handler to work around the lack of preprocessor support, but it's probably still a good idea to pass more details along anyways.

Conduitry commented 5 years ago

Yeah, I'm definitely into this in theory. I'd considered this in the past, but it doesn't play entirely nicely with the current settings stuff. I'm trying to make most of the settings be at least somewhat usable without having them be set to a callback - that requires having a .eslintrc.js file, which I think is still way less common than a JSON or YAML one.

The callback currently only gets the code of the warning, because when the setting is an array, it's taken as an array of codes to ignore.

The simplest way to add this (and one that would still be backwards compatible) that I can think of is to add the entire warning object as a second argument to the ignore-warnings callback, in addition to the first argument which will still be its code. This is a bit clunky, but it a) wouldn't be a breaking change, and b) would still also allow the array-of-codes form to work in a sensible way.

Thoughts?

tivac commented 5 years ago

That's how I'd add support for this, yes. It's a tiny bit clunky but worth it to continue supporting the folks stuck in the dark ages of JSON/YAML eslint configs.

Conduitry commented 5 years ago

This has been added in v1.2.0, which the entire warning object passed as a second argument.