search

sveltejs / eslint-plugin-svelte3

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

Parse error inside <style> block #3

Closed maxmilton closed 5 years ago

maxmilton commented 5 years ago

A parse error is thrown when parsing <style> block with non-standard CSS:

repro-styles

With normal CSS I do like that the plugin identifies unused selectors. Looks like we might either need a way to turn off style parsing or support preprocessors.

Preprocessor support would be ideal (I suspect some users might even use markup preprocessors, e.g. jade) but obviously it adds complexity to this plugin. Preprocessors would need to return a valid sourcemap and the plugin would need to take it into account. There's also the fact that there's no standard place to declare preprocessors — either we need something like a standardised svelte.config.js (big/controversial upstream change) or some extra plugin options to specify preprocessors (requires maintaining 2 places). Neither of those options sound particularly great to me.

maxmilton commented 5 years ago

After actually looking at the plugin source, it looks like this plugin simply piggybacks on the output generated by svelte. I'm not sure what the state of source map handling is in svelte at the moment but it might be easier than I expect to get preprocessor support.

I'll experiment with the plugin and see if passing in preprocessors is feasible.

Conduitry commented 5 years ago

I'm not sure what to do with this either. I don't know what a good way to pass in a preprocessor via eslintrc would be. And then even if there was a reasonable way to do that, we'd have to decode the source maps and map any warnings and errors back (which is probably as much a moral quandary as it is a technical one - I really would prefer not to introduce a whole sourcemap library as a dependency).

In any case, it seems likely that CSS warnings generated by Svelte about unused styles wouldn't be able to be mapped back nicely to the source postcss/less/sass/whatever. I'm wondering whether it might be better to just provide a way to completely ignore style blocks. Under the hood, this would mean removing or blanking out the style tag and its contents before passing the component on to the compiler. You'd miss out on in-editor warnings about unused styles, but they'd still show up in the logs of your actual build. Does this sound reasonable? One thing that is not clear to me right now is how the ignore-style-blocks thing would be configurable in eslintrc. An all or nothing thing? By the presence of a given attribute? By certain attributes having certain values? None of the logic here is going to be particularly complicated, but I don't know how to represent a reasonable array of options in eslintrc settings.

Conduitry commented 5 years ago

There actually is a reasonable way to include arbitrary functions in the ESLint config (thanks @TehShrike!) - just use a CommonJS-formatted .eslintrc.js file and include the function. This provides a powerful way to have whatever necessary logic when deciding whether to wipe a style block in a component. We can still allow a boolean value (possible in JSON and YAML) for an all-or-nothing option.

Conduitry commented 5 years ago

There's an initial version of this available if you install from the ignore-styles branch. There's a new setting svelte3/ignore-styles that you can set to a boolean or to a function that accepts an object of attribute names+values and returns a boolean.

Conduitry commented 5 years ago

svelte3/ignore-styles has been added in v0.4.0, along with some other (breaking) changes that make configuration a little more flexible.