Skip to content

Commit

Permalink
fix(parse): mutable highlight bounds (#6419)
Browse files Browse the repository at this point in the history
This PR optimizes highlight bounds manipulation by:
1. Making the `highlightBounds` mutable *only when being edited* - to
avoid re-creating them when inside loops
2. Replaces `for-of` implementation with `reduce` and mutable
assignment, which is a bit more performant:
(going with option 2 instead of the current 1):
<img width="981" alt="image"
src="https://github.com/user-attachments/assets/33d14062-6983-4fec-a19e-83103fb5308a">

**Manual Tests:**
I hereby swear that:

- [X] I opened a hydrogen project and it loaded
- [X] I could navigate to various routes in Preview mode
  • Loading branch information
liady authored Sep 29, 2024
1 parent fde6292 commit 40e6424
Showing 1 changed file with 20 additions and 18 deletions.
38 changes: 20 additions & 18 deletions editor/src/core/workers/parser-printer/parser-printer-parsing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,7 @@ function parseOtherJavaScript<T extends { uid: string }>(
}

let parsedElementsWithin: ElementsWithinInPosition = []
let highlightBounds = existingHighlightBounds
let highlightBounds_MUTABLE = { ...existingHighlightBounds }

function addToParsedElementsWithin(
currentScope: Array<string>,
Expand All @@ -1114,7 +1114,7 @@ function parseOtherJavaScript<T extends { uid: string }>(
imports,
topLevelNames,
propsObjectName,
highlightBounds,
highlightBounds_MUTABLE,
alreadyExistingUIDs,
applySteganography,
)
Expand All @@ -1133,7 +1133,10 @@ function parseOtherJavaScript<T extends { uid: string }>(
? firstChild.startColumn - startColumnShift
: firstChild.startColumn,
})
highlightBounds = mergeHighlightBounds(highlightBounds, success.highlightBounds)
highlightBounds_MUTABLE = mergeHighlightBounds(
highlightBounds_MUTABLE,
success.highlightBounds,
)

fastForEach(success.definedElsewhere, (val) =>
pushToDefinedElsewhereIfNotThere(currentScope, val),
Expand Down Expand Up @@ -1513,11 +1516,11 @@ function parseOtherJavaScript<T extends { uid: string }>(

return mapEither((created) => {
// Add in the bounds for the entire value.
highlightBounds = addToHighlightBounds(
highlightBounds,
addToHighlightBoundsMutate(
highlightBounds_MUTABLE,
buildHighlightBoundsForExpressionsAndText(sourceFile, expressionsAndTexts, created.uid),
)
return withParserMetadata(created, highlightBounds, propsUsed, definedElsewhere)
return withParserMetadata(created, highlightBounds_MUTABLE, propsUsed, definedElsewhere)
}, failOnNullResult(create(code, definedWithin, definedElsewhere, fileNode, parsedElementsWithin, paramsToUse, statements)))
}
}
Expand Down Expand Up @@ -2995,11 +2998,13 @@ function buildHighlightBoundsForUids(
function mergeHighlightBounds(
...multipleBounds: Array<HighlightBoundsForUids>
): Readonly<HighlightBoundsForUids> {
let result: HighlightBoundsForUids = {}
const acc: HighlightBoundsForUids = {}
for (const bounds of multipleBounds) {
Object.assign(result, bounds)
for (const key in bounds) {
acc[key] = bounds[key]
}
}
return result
return acc
}

function merge2WithParserMetadata<T1, T2, U>(
Expand Down Expand Up @@ -3032,17 +3037,14 @@ function addBoundsIntoWithParser<T>(
}
}

function addToHighlightBounds(
existing: Readonly<HighlightBoundsForUids>,
function addToHighlightBoundsMutate(
existing: HighlightBoundsForUids,
toAdd: HighlightBounds | null,
): Readonly<HighlightBoundsForUids> {
if (toAdd == null) {
return existing
} else {
let result: HighlightBoundsForUids = { ...existing }
result[toAdd.uid] = toAdd
return result
): HighlightBoundsForUids {
if (toAdd != null) {
existing[toAdd.uid] = toAdd
}
return existing
}

interface UpdateUIDResult {
Expand Down

0 comments on commit 40e6424

Please sign in to comment.