Handle caught 'SyntaxError' errors

[ericberens]

• Updated processor to return the original input on SyntaxError errors so it can fall through to other processors

[ericberens]

Ideally, stylelint directly should handle whether or not a processor should process a particular file but since that is not supported I'd like to be able to pass off the input to other processors in the case where the stylelint-processor-styled-components is unable to parse the input.

Originally I solved this by running separate npm command

stylelint --config css-stylelint.config.js '**/*.css'
and
stylelint --config styled-components-stylelint.config.js '**/*.{js,jsx,ts,tsx}'

but that was problematic trying to get https://github.com/shinnn/vscode-stylelint to work since it requires a single configuration and I was stuck choosing which type of implementation I wanted to lint at the IDE level.

[coveralls]

Coverage decreased (-0.7%) to 98.917% when pulling 180770b on ericberens:master into 3533d05 on styled-components:master.

[emilgoldsmith]

Interesting, so I'm assuming that what you did here works right? Could you add some tests please? But yeah this would be a very nice feature to have

[emilgoldsmith]

Adding a comment above here would be awesome too

[ericberens]

Will add a comment and update PR

[chinesedfan]

@ericberens By the way, how about checking e.name directly, without a new local variable declared?

[emilgoldsmith]

I agree, especially because we use e.name in the conditional above, that's odd and inconsistent

[jiahaog]

I'm wondering if it's necessary to even do the name === 'SyntaxError' check, will it be sufficient to just return input on all errors and let the stylelint engine process it?

[ericberens]

@emilgoldsmith yes, returning the input in an explicit error branch or in general as a replacement to '' allows for the input to fall through to any additional configured processors or if no additional processors are configured assumed (by default usage) that it is vanilla CSS.

Will add a comment regarding the additional conditional statement and look to add test to cover this scenario.

[emilgoldsmith]

Sounds good.

Yeah my biggest question was also whether the SyntaxError error type is correct, but I guess that's what Babylon throws in case it doesn't find Javascript? Make sure that it also encompasses the typescript-eslint-parser for when we use Typescript (even though we're soon migrating to Babylon fully anyway when v7 is released, but for now we should stay compatible), and that it also throws SyntaxError.

But yeah otherwise I think I like this implementation as long as the if statement catches exactly the cases we want it to. As anything non-javascript shouldn't be passed to our processor anyway, so if it's CSS it'll just be processed and if it's a third language the rest of Stylelint can throw whatever errors it should as either there's another processor (as you mention) or someone accidentally gave an invalid file as input

[jiahaog]

I was wondering why my global stylesheets weren't linting properly, and it turns out that there's a return '' when an error is caught, and stylelint-config-styled-components already disables the no-empty-source rule so it just skips linting of those global stylesheets silently.

@ericberens are you still on this PR? I can help to update it with the comments if you're unable to do so 😄!

[emilgoldsmith]

Yeah I mention it in this issue: #155, though it should probably have been a separate issue. Feel free to make it throw a CssSyntaxError though that Stylelint can understand, and update the PR to do that.

As to your suggestion of just returning input on every single error I don't think I like though, since it'll just make Stylelint output cryptic errors as it can't read javascript and that's the kind of errors that will come out.

The reason for this PR I believe (though it's been a while since I last looked at it) is to allow people to lint both css files and CSS-in-JS files with our processor, correct @ericberens ?
This has some edge cases too though as if you for example write some incorrect JS it'll again make Stylelint output cryptic errors as it gets JS to process. What about we 1. throw an error on non CssSyntaxError saying something like an unexpected error occurred in the Stylelint Processor for Styled Components, and instead of this SyntaxError check simply at the start of this function call, do a quick regex to check whether the extension of this file is \.jsx?|\.ts? If it's not we simply return input, otherwise we proceed with our processing?

@jiahaog if you want to help out feel free to update this one or just submit a new PR

[rylek90]

Any chance it'll be merged soon? 😄

[emilgoldsmith]

Any chance it'll be merged soon? smile

If someone finishes it I guess

[chinesedfan]

Closed by #211.