-
-
Notifications
You must be signed in to change notification settings - Fork 2k
perf: reduce some repeated logic #4814
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
Conversation
To avoid duplicating logic, we can use `createElement` with pre-populated props.
📊 Tachometer Benchmark ResultsSummary⏳ Benchmarks are currently running. Results below are out of date. duration
usedJSHeapSize
Results⏳ Benchmarks are currently running. Results below are out of date. create10kduration
usedJSHeapSize
filter-listduration
usedJSHeapSize
hydrate1kduration
usedJSHeapSize
many-updatesduration
usedJSHeapSize
replace1kduration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-warmup-3
run-warmup-4
run-final
text-updateduration
usedJSHeapSize
tododuration
usedJSHeapSize
update10th1kduration
usedJSHeapSize
|
|
This increases byte size as you can see in https://github.com/preactjs/preact/actions/runs/15876304094/job/44764641936?pr=4814 the bot sadly can't post messages for forks 😅 The action can be clicked and inspected though (compressed size) |
|
I'll look into it as it's smaller in the bundle locally 🤔 so must be minification differences somehow |
|
its actually the cloneElement change i think, because it was literally the same duplicated code before, it'd compress better! but the actual JS build output is smaller 😬 |
|
@JoviDeCroock what i mean is the files on disk are smaller, and the logic should be faster (but micro optimisation i guess) the compressed files are larger. presumably because ill see if there's any way i can reduce it |
|
I reckon that reverting the |
|
JoviDeCroock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really clever, it basically flattens the loop
* perf: use createElement when cloning To avoid duplicating logic, we can use `createElement` with pre-populated props. * chore: reduce search logic * perf: reuse matched flag * chore: put the function inline again * perf: simplify to a ternary * perf: revert clone function --------- Co-authored-by: Jovi De Croock <[email protected]>
* perf: use createElement when cloning To avoid duplicating logic, we can use `createElement` with pre-populated props. * chore: reduce search logic * perf: reuse matched flag * chore: put the function inline again * perf: simplify to a ternary * perf: revert clone function --------- Co-authored-by: James Garbutt <[email protected]>
this reduces the bundle size a little (few hundred bytes i think)
do we have a bot or a tool to diff the sizes?