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

Replace CSS parser with postcss #40

Merged
merged 1 commit into from
Sep 14, 2020
Merged

Conversation

jdsutherland
Copy link
Contributor

@jdsutherland jdsutherland commented Sep 6, 2020

Partially fixes #36.

https://www.npmjs.com/package/css doesn't support newer CSS features. When encountering newer CSS like CSS3 variables or certain grid syntax, the server blows up because the parser returns undefined. postcss doesn't have this issue.

There is an additional wrinkle here as mentioned in #36:

csslint doesn't support some newer features, notably CSS3 variables. So even though postcss works, csslint causes bracey to show false positives.

so with this PR, the parsing works but csslint will still cause bracey to blow up.

It looks like csslint is dead (see CSSLint/csslint#720), so the solution appears to be either replace it with stylelint or simply remove csslint from bracey.

I'm not sure this PR will get any response but if there is interest, we can discuss the linting issues.

var source = rule.source;
source.start.line--;
source.start.column--;
source.end.column++;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't fully understand why this was done previously so I made this equivalent.

Copy link
Owner

@turbio turbio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great! Thanks for fixing this, I'll merge it in as soon as I get a chance to test it.

@turbio turbio merged commit b9ed30c into turbio:master Sep 14, 2020
@jdsutherland
Copy link
Contributor Author

jdsutherland commented Sep 14, 2020

@turbio the issue still persists because csslint doesn't support newer CSS syntax. This PR merge is progress but doesn't have much effect without addressing the linting (it also blows up). What are your thoughts?

@turbio
Copy link
Owner

turbio commented Sep 18, 2020

@jdsutherland gotcha, I'll re-open the issue. I'm thinking the best path forward is to update the linter as well (assuming that's easy). I forget the specifics of how linting works in Bracey but for the current purpose it might be sufficient to also use the postcss parser for error checking.

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.

Blows up with CSS Grid Layout
2 participants