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.

Sign up for GitHub

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

Jump to bottom

perf(vite): rework endsWith to direct index #24746

Merged
merged 9 commits into from
Jan 29, 2024
Merged

perf(vite): rework endsWith to direct index #24746

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7112720
perf(vite): rework endsWith to direct index checking
GalacticHypernova Dec 14, 2023
347ebc6
perf(vite): use trimEnd
GalacticHypernova Dec 14, 2023
bae8777
Merge branch 'main' into patch-18
GalacticHypernova Dec 16, 2023
bbe112b
Merge branch 'main' into patch-18
GalacticHypernova Dec 17, 2023
02dda29
Merge branch 'main' into patch-18
GalacticHypernova Dec 20, 2023
8ee9adc
chore: revert trimEnd
GalacticHypernova Dec 28, 2023
074973f
Merge branch 'main' into patch-18
GalacticHypernova Dec 28, 2023
b068748
Merge branch 'nuxt:main' into patch-18
GalacticHypernova Jan 5, 2024
33dc5a0
Merge branch 'main' into patch-18
GalacticHypernova Jan 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/vite/src/plugins/composable-keys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ export const composableKeysPlugin = createUnplugin((options: ComposableKeysOptio
}

// TODO: Optimize me (https://github.com/nuxt/framework/pull/8529)
const endsWithComma = code.slice(codeIndex + (node as any).start, codeIndex + (node as any).end - 1).trim().endsWith(',')
const newCode = code.slice(codeIndex + (node as any).start, codeIndex + (node as any).end - 1).trim()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danielroe this entire slice/trim operation, is it even needed? Can we not just do this?

code[codeIndex + (node as any).end - 1]===','

I saw that pi left a comment there, is there a possibility codeIndex + (node as any).end - 1 would point to a whitespace?

Copy link
Member

Choose a reason for hiding this comment

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

yes, I think it's possible it will end with whitespace...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I'll try to run some tests with this. There's gotta be a way to make it work without it..

Copy link
Member

Choose a reason for hiding this comment

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

you could iterate backwards in a while loop... 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That could be possible, it will at the very least reduce the amount of characters "digested" by trim I suppose which runs both ways (will make some benchmarks for this), but I wonder if there's a way to make it even faster, like if the expression itself will not contain whitespace, in which case last index could be used directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

For starter optimization, we can use trimEnd which increase performance by ~50%

const endsWithComma = newCode[newCode.length - 1] === ','

s.appendLeft(
codeIndex + (node as any).end - 1,
Expand Down