Skip to content

Conversation

@ematipico
Copy link
Member

@ematipico ematipico commented Jul 1, 2025

Summary

Part of #6657

This PR introduces two new parsing strategies for JS and CSS, allowing us to parse a document and set an offset. These parsing strategies are useful for supporting JavaScript and CSS inside HTML documents and keeping the same text range of the original content.

The PR also adds a potential solution for storing the information. The Document type, inside biome_service now stores new types called SendJsEmbeddedParse and SendCssEmbeddedParse. These types are strongly typed, Sync and Send nodes of new types called JsOffsetParse and CssOffsetParse

Internally, these Send* types use the new type EmbeddedSendNode, which stores the offset.

Test Plan

Added new tests

@changeset-bot
Copy link

changeset-bot bot commented Jul 1, 2025

⚠️ No Changeset found

Latest commit: cc72f37

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added A-Core Area: core A-Project Area: project A-Parser Area: parser L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS L-JSON Language: JSON and super languages labels Jul 1, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2025

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 50611 50611 0
Passed 49294 49294 0
Failed 1317 1317 0
Panics 0 0 0
Coverage 97.40% 97.40% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6706 6706 0
Passed 2244 2244 0
Failed 4462 4462 0
Panics 0 0 0
Coverage 33.46% 33.46% 0.00%

ts/babel

Test result main count This PR count Difference
Total 822 822 0
Passed 731 731 0
Failed 91 91 0
Panics 0 0 0
Coverage 88.93% 88.93% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 18783 18783 0
Passed 14411 14411 0
Failed 4372 4372 0
Panics 0 0 0
Coverage 76.72% 76.72% 0.00%

@ematipico ematipico force-pushed the feat/offset-parsing branch from 028bc3d to eedab2b Compare July 1, 2025 08:27
@codspeed-hq
Copy link

codspeed-hq bot commented Jul 1, 2025

CodSpeed Performance Report

Merging #6652 will not alter performance

Comparing feat/offset-parsing (cc72f37) with main (ef2421d)

Summary

✅ 114 untouched benchmarks

@ematipico ematipico marked this pull request as ready for review July 1, 2025 15:37
@ematipico ematipico requested review from a team July 1, 2025 15:37
Copy link
Contributor

@arendjr arendjr left a comment

Choose a reason for hiding this comment

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

Sweet!


// Walk through all HTML elements looking for script tags
for element in html_root.syntax().descendants() {
let Some(html_element) = HtmlElement::cast(element.clone()) else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: If we put the entire inner body for the loop inside of a separate extract_embedded_script() function (note it's singular), I believe we can get rid of all these continue statements, and the extra indentation of the last if statement may be avoidable too.

Also, I guess HtmlElement::cast_ref() is a better choice here?

Comment on lines +408 to +410
if matches!(file_source, DocumentFileSource::Html(_)) {
if let Some(Ok(any_parse)) = &syntax {
if let Some(html_root) =
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it's time for Rust 1.88's let chains :D

@ematipico ematipico merged commit e581f6d into main Jul 2, 2025
29 checks passed
@ematipico ematipico deleted the feat/offset-parsing branch July 2, 2025 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Core Area: core A-Parser Area: parser A-Project Area: project L-CSS Language: CSS L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants