-
-
Notifications
You must be signed in to change notification settings - Fork 2k
refactor(props): call toLowerCase on name regardless of its current casing #5003
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
📊 Tachometer Benchmark ResultsSummaryduration
usedJSHeapSize
Resultscreate10kduration
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
|
rschristian
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.
The benches almost certainly don't cover this situation, the numbers returned shouldn't be too useful.
This PR just trades perf from lowercased event names as props and gives it to camelcased. Latter is probably more popular, so maybe this is a positive change?
| else if (name[0] == 'o' && name[1] == 'n') { | ||
| useCapture = name != (name = name.replace(CAPTURE_REGEX, '$1')); | ||
|
|
||
| // Infer correct casing for DOM built-in events: |
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.
Not something that needs to be done here, but we probably should do away with this comment now that we're not inferring anything (and instead just assuming the vent matches DOM event conventions).
if benchmarks don't get negatively affected, we could save some bytes and perhaps even improve performance a bit