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

☂️ CSS Parser #268

Closed
31 tasks done
ematipico opened this issue Sep 13, 2023 · 35 comments
Closed
31 tasks done

☂️ CSS Parser #268

ematipico opened this issue Sep 13, 2023 · 35 comments
Assignees
Labels
A-Parser Area: parser L-CSS Language: CSS S-Help-wanted Status: you're familiar with the code base and want to help the project

Comments

@ematipico
Copy link
Member

ematipico commented Sep 13, 2023

Description

This is an umbrella issue to track the works of the CSS parser.

The lexer of the CSS parser should be ready, so now it's time to start working on the parser itself.

We should already have a basic syntax, although it's possible that some grammar or cases are not covered, so it's highly* suggested to always make sure we have to nodes to represent a specific grammar.

Resources:

Let's remember that Biome parser needs to be recoverable, so we have to consider invalid cases and ensure that the parser can recover itself and resume parsing in a fancy matter.

How to contribute

Before starting, please wait for us to implement the testing infrastructure. In the meantime, you can take a look at the JS parser and JSON parser to understand how the parsing works, the internal APIs, the recovery system, etc.

In this very issue, add a comment saying you want to work on a specific task(s). We will create the issue for you and eventually assign it to:

Note

In order to have the issue assigned to you, you must comment on the issue.,

Prior work

You can the previous Rome's (TS) implementation of the parser: https://github.com/rome/tools/blob/archived-js/internal/css-parser/parser/rules.ts

Before starting

  • Create a new test infrastructure for the CSS parser
  • Write instructions to run the CSS test infra

CSS parser tasks

Parsing at-rules

At-rules are very strict, and they have a determined syntax. Let us adhere to that.

Ideas

... more to come

@ematipico ematipico added A-Parser Area: parser L-CSS Language: CSS labels Sep 13, 2023
@ematipico ematipico added this to the Someday milestone Sep 13, 2023
@Conaclos Conaclos added the S-Help-wanted Status: you're familiar with the code base and want to help the project label Sep 14, 2023
@kubijo
Copy link

kubijo commented Sep 23, 2023

Forgive me if this is a dumb question, but isn't there a chance for you to reuse a lot of what lightningcss does? As far as I know they use some rust crates written by Mozilla for Firefox, so it should be a very solid base...

@ematipico
Copy link
Member Author

Forgive me if this is a dumb question, but isn't there a chance for you to reuse a lot of what lightningcss does? As far as I know they use some rust crates written by Mozilla for Firefox, so it should be a very solid base...

We did consider it. While doing some research we reached out to Devon, and he suggested to not use that crate (the Mozilla one) because it wouldn't fit our CST.

I also created a PoC some time ago, and I can confirm that unfortunately that crate has some limitations for us.

@kubijo
Copy link

kubijo commented Sep 23, 2023

Oh… ok, that's a shame. So there is no prior art for you then, given your architecture?

@denbezrukov
Copy link
Contributor

I'm going to return to this task the next monday🙃

denbezrukov added a commit that referenced this issue Sep 27, 2023
- CssIdSelectorPattern
- CssClassSelectorPattern
- CssTypeSelectorPattern
denbezrukov added a commit that referenced this issue Sep 30, 2023
Introduce new grama for selectors.
denbezrukov added a commit that referenced this issue Sep 30, 2023
denbezrukov added a commit that referenced this issue Oct 5, 2023
denbezrukov added a commit that referenced this issue Oct 6, 2023
denbezrukov added a commit that referenced this issue Oct 6, 2023
denbezrukov added a commit that referenced this issue Oct 6, 2023
denbezrukov added a commit that referenced this issue Oct 6, 2023
denbezrukov added a commit that referenced this issue Oct 7, 2023
denbezrukov added a commit that referenced this issue Oct 10, 2023
@denbezrukov
Copy link
Contributor

I've updated the issue to start parsing properties.
source: https://github.com/parcel-bundler/lightningcss/tree/master/src/properties

@faultyserver
Copy link
Contributor

I created #1359 to track implementing the CSS-wide keywords as tokens and making them invalid as custom identifiers.

@faultyserver
Copy link
Contributor

I've gotten part way through implementing some infrastructure to enable exact parsing of individual properties, but I'm realizing that it will require some changes to the parser itself to support checkpoint rewinding (similar to how the JS parser already works), since there's ambiguity about whether a parsed value is valid until reaching the end of the value definition (i.e., the ; at the end of the line). I'll probably try to build out that checkpointing in a separate PR first and then utilize it in this work, and then from there we should be able to start parsing a lot of properties directly!

I'll open an issue for tracking that work either later tonight or tomorrow.

@denbezrukov
Copy link
Contributor

#1472 (comment)

@Gami13
Copy link

Gami13 commented Feb 14, 2024

Could I start working on implementing the other properties in a matter similar to the border one or are you working on a better solution?

@ematipico
Copy link
Member Author

@Gami13 do you mean this issue? #1454

If so, yes, feel free to comment the issue and I will assign it to you

@denbezrukov
Copy link
Contributor

I think that we want to keep property parsing as it is now. We want to have properties at a semantic level.

cc @faultyserver

@denbezrukov
Copy link
Contributor

@Gami13
You can try to implement #1333 if you want.

@Gami13
Copy link

Gami13 commented Feb 14, 2024

@ematipico yeah, exactly but for the other properties like align, background etc. assuming the way its done in the border one is close to final, I could work on implementing it for the rest

@faultyserver
Copy link
Contributor

assuming the way its done in the border one is close to final, I could work on implementing it for the rest

@Gami13 as Denis mentioned, after investigating with #1453 and discussing afterward, we are most likely going to keep the parser generic, just parsing component value lists for each property, and then have the analyzers do the actual semantic parsing to get the real values for each rule. There's just too much context and evaluation needed for this to be feasible in a parser, and it will be much easier with an evaluation engine in the analyzer as well.

So, unfortunately, please ignore how the border property is being parsed, it will be removed eventually and returned to being generic. The best place to add new features in the parser would be the remaining @-rules like #1333 that Denis mentioned

@bjoroen
Copy link
Contributor

bjoroen commented Feb 16, 2024

Hello,
I would love to get more involved with this project, is there any of the properties issues that would be good for me to start with? Or is there anything else that needs to be done?

@denbezrukov
Copy link
Contributor

denbezrukov commented Feb 16, 2024

@bjoroen
I believe it's better to look at #1285
However, you could take #1743

@bjoroen
Copy link
Contributor

bjoroen commented Feb 16, 2024

@denbezrukov I could take #1743 if it's possible.

@bjoroen
Copy link
Contributor

bjoroen commented Feb 16, 2024

You can assign #1743 to me, and I'll take a look at it tomorrow.

@ematipico
Copy link
Member Author

@bjoroen you have to comment on the issue, so we can assign that to you. That's how GitHub works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Parser Area: parser L-CSS Language: CSS S-Help-wanted Status: you're familiar with the code base and want to help the project
Projects
None yet
Development

No branches or pull requests