Skip to content

Revert unkeyed no-search#4604

Merged
JoviDeCroock merged 2 commits intomainfrom
revert-hole-unmount
Dec 12, 2024
Merged

Revert unkeyed no-search#4604
JoviDeCroock merged 2 commits intomainfrom
revert-hole-unmount

Conversation

@JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Dec 11, 2024

Reverts #4550

This revert isn't necessarily needed but after a lot of investigation we do increase components remounting a lot, to avoid people having to deal with a breaking change like that I'd like to propose a revert here.

If people fall into this issue, please use key to avoid elements being reused when you don't want to.

@github-actions
Copy link

github-actions bot commented Dec 11, 2024 •
edited
Loading

📊 Tachometer Benchmark Results

Summary

duration

  • create10k: unsure 🔍 -1% - +0% (-6.50ms - +1.94ms)
    preact-local vs preact-main
  • filter-list: unsure 🔍 -1% - +0% (-0.09ms - +0.04ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 -2% - +2% (-1.41ms - +1.13ms)
    preact-local vs preact-main
  • many-updates: unsure 🔍 -0% - +2% (-0.04ms - +0.35ms)
    preact-local vs preact-main
  • replace1k: unsure 🔍 -1% - +1% (-0.68ms - +0.70ms)
    preact-local vs preact-main
  • text-update: unsure 🔍 -5% - +5% (-0.10ms - +0.09ms)
    preact-local vs preact-main
  • todo: unsure 🔍 -1% - +1% (-0.25ms - +0.23ms)
    preact-local vs preact-main
  • update10th1k: unsure 🔍 -4% - +2% (-1.23ms - +0.63ms)
    preact-local vs preact-main

usedJSHeapSize

  • create10k: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • filter-list: unsure 🔍 -1% - +0% (-0.02ms - +0.01ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 -2% - +4% (-0.15ms - +0.43ms)
    preact-local vs preact-main
  • many-updates: unsure 🔍 -0% - +1% (-0.01ms - +0.03ms)
    preact-local vs preact-main
  • replace1k: unsure 🔍 -1% - +0% (-0.03ms - +0.00ms)
    preact-local vs preact-main
  • text-update: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • todo: unsure 🔍 -0% - -0% (-0.00ms - -0.00ms)
    preact-local vs preact-main
  • update10th1k: unsure 🔍 -0% - +0% (-0.01ms - +0.01ms)
    preact-local vs preact-main

Results

create10k

duration

VersionAvg timevs preact-localvs preact-main
preact-local940.86ms - 945.56ms-unsure 🔍
-1% - +0%
-6.50ms - +1.94ms
preact-main941.98ms - 949.00msunsure 🔍
-0% - +1%
-1.94ms - +6.50ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local19.21ms - 19.21ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main19.21ms - 19.21msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-
filter-list

duration

VersionAvg timevs preact-localvs preact-main
preact-local16.55ms - 16.62ms-unsure 🔍
-1% - +0%
-0.09ms - +0.04ms
preact-main16.56ms - 16.67msunsure 🔍
-0% - +1%
-0.04ms - +0.09ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.52ms - 1.53ms-unsure 🔍
-1% - +0%
-0.02ms - +0.01ms
preact-main1.52ms - 1.54msunsure 🔍
-0% - +1%
-0.01ms - +0.02ms		-
hydrate1k

duration

VersionAvg timevs preact-localvs preact-main
preact-local66.62ms - 68.02ms-unsure 🔍
-2% - +2%
-1.41ms - +1.13ms
preact-main66.40ms - 68.52msunsure 🔍
-2% - +2%
-1.13ms - +1.41ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local10.03ms - 10.45ms-unsure 🔍
-2% - +4%
-0.15ms - +0.43ms
preact-main9.90ms - 10.30msunsure 🔍
-4% - +1%
-0.43ms - +0.15ms		-
many-updates

duration

VersionAvg timevs preact-localvs preact-main
preact-local18.96ms - 19.25ms-unsure 🔍
-0% - +2%
-0.04ms - +0.35ms
preact-main18.83ms - 19.07msunsure 🔍
-2% - +0%
-0.35ms - +0.04ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local3.76ms - 3.79ms-unsure 🔍
-0% - +1%
-0.01ms - +0.03ms
preact-main3.75ms - 3.78msunsure 🔍
-1% - +0%
-0.03ms - +0.01ms		-
replace1k
  • Browser: chrome-headless
  • Sample size: 100
  • Built by: CI #4192

  • Commit: 5d8d22b

duration

VersionAvg timevs preact-localvs preact-main
preact-local67.04ms - 68.29ms-unsure 🔍
-1% - +1%
-0.68ms - +0.70ms
preact-main67.36ms - 67.95msunsure 🔍
-1% - +1%
-0.70ms - +0.68ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local2.96ms - 2.98ms-unsure 🔍
-1% - +0%
-0.03ms - +0.00ms
preact-main2.97ms - 3.00msunsure 🔍
-0% - +1%
-0.00ms - +0.03ms		-

run-warmup-0

VersionAvg timevs preact-localvs preact-main
preact-local30.05ms - 30.47ms-unsure 🔍
-1% - +1%
-0.45ms - +0.27ms
preact-main30.06ms - 30.64msunsure 🔍
-1% - +1%
-0.27ms - +0.45ms		-

run-warmup-1

VersionAvg timevs preact-localvs preact-main
preact-local35.10ms - 36.38ms-unsure 🔍
-3% - +2%
-1.09ms - +0.65ms
preact-main35.37ms - 36.56msunsure 🔍
-2% - +3%
-0.65ms - +1.09ms		-

run-warmup-2

VersionAvg timevs preact-localvs preact-main
preact-local26.16ms - 26.49ms-unsure 🔍
-1% - +0%
-0.34ms - +0.12ms
preact-main26.27ms - 26.59msunsure 🔍
-0% - +1%
-0.12ms - +0.34ms		-

run-warmup-3

VersionAvg timevs preact-localvs preact-main
preact-local25.79ms - 27.13ms-unsure 🔍
-2% - +5%
-0.43ms - +1.38ms
preact-main25.38ms - 26.59msunsure 🔍
-5% - +2%
-1.38ms - +0.43ms		-

run-warmup-4

VersionAvg timevs preact-localvs preact-main
preact-local24.28ms - 25.49ms-unsure 🔍
-2% - +5%
-0.38ms - +1.30ms
preact-main23.84ms - 25.01msunsure 🔍
-5% - +2%
-1.30ms - +0.38ms		-

run-final

VersionAvg timevs preact-localvs preact-main
preact-local21.87ms - 22.31ms-unsure 🔍
-1% - +1%
-0.30ms - +0.25ms
preact-main21.95ms - 22.29msunsure 🔍
-1% - +1%
-0.25ms - +0.30ms		-
text-update
  • Browser: chrome-headless
  • Sample size: 170
  • Built by: CI #4192

  • Commit: 5d8d22b

duration

VersionAvg timevs preact-localvs preact-main
preact-local1.96ms - 2.06ms-unsure 🔍
-5% - +5%
-0.10ms - +0.09ms
preact-main1.93ms - 2.09msunsure 🔍
-5% - +5%
-0.09ms - +0.10ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.11ms - 1.11ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main1.11ms - 1.11msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-
todo

duration

VersionAvg timevs preact-localvs preact-main
preact-local33.73ms - 33.95ms-unsure 🔍
-1% - +1%
-0.25ms - +0.23ms
preact-main33.63ms - 34.06msunsure 🔍
-1% - +1%
-0.23ms - +0.25ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.22ms - 1.23ms-unsure 🔍
-0% - -0%
-0.00ms - -0.00ms
preact-main1.22ms - 1.23msunsure 🔍
+0% - +0%
+0.00ms - +0.00ms		-
update10th1k

duration

VersionAvg timevs preact-localvs preact-main
preact-local30.87ms - 32.25ms-unsure 🔍
-4% - +2%
-1.23ms - +0.63ms
preact-main31.24ms - 32.49msunsure 🔍
-2% - +4%
-0.63ms - +1.23ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local2.91ms - 2.92ms-unsure 🔍
-0% - +0%
-0.01ms - +0.01ms
preact-main2.91ms - 2.92msunsure 🔍
-0% - +0%
-0.01ms - +0.01ms		-

tachometer-reporter-action v2 for CI

JoviDeCroock
JoviDeCroock commented Dec 11, 2024
View reviewed changes
test/browser/render.test.js
});

// #2949
it.skip('should not swap unkeyed chlildren', () => {
Copy link
Member Author

@JoviDeCroock JoviDeCroock Dec 11, 2024

Choose a reason for hiding this comment

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

Added a test highlighting the issue

@github-actions
Copy link

github-actions bot commented Dec 11, 2024 •
edited
Loading

Size Change: -68 B (-0.11%)

Total Size: 62.3 kB

Filename Size Change
dist/preact.js 4.7 kB -15 B (-0.32%)
dist/preact.min.js 4.72 kB -8 B (-0.17%)
dist/preact.min.module.js 4.72 kB -9 B (-0.19%)
dist/preact.min.umd.js 4.74 kB -12 B (-0.25%)
dist/preact.module.js 4.72 kB -11 B (-0.23%)
dist/preact.umd.js 4.76 kB -13 B (-0.27%)
ℹ️ View Unchanged
Filename Size
compat/dist/compat.js 4.13 kB
compat/dist/compat.module.js 4.06 kB
compat/dist/compat.umd.js 4.2 kB
debug/dist/debug.js 3.82 kB
debug/dist/debug.module.js 3.83 kB
debug/dist/debug.umd.js 3.9 kB
devtools/dist/devtools.js 260 B
devtools/dist/devtools.module.js 274 B
devtools/dist/devtools.umd.js 346 B
hooks/dist/hooks.js 1.52 kB
hooks/dist/hooks.module.js 1.56 kB
hooks/dist/hooks.umd.js 1.6 kB
jsx-runtime/dist/jsxRuntime.js 973 B
jsx-runtime/dist/jsxRuntime.module.js 947 B
jsx-runtime/dist/jsxRuntime.umd.js 1.05 kB
test-utils/dist/testUtils.js 451 B
test-utils/dist/testUtils.module.js 456 B
test-utils/dist/testUtils.umd.js 536 B

compressed-size-action

@coveralls
Copy link

coveralls commented Dec 11, 2024 •
edited
Loading

Coverage Status

coverage: 99.615% (-0.002%) from 99.617%
when pulling 5d8d22b on revert-hole-unmount
into eba80f7 on main.

@JoviDeCroock JoviDeCroock merged commit fcb8d7c into main Dec 12, 2024
@JoviDeCroock JoviDeCroock deleted the revert-hole-unmount branch December 12, 2024 07:01
@JoviDeCroock JoviDeCroock mentioned this pull request Dec 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rschristian rschristian rschristian approved these changes

@developit developit Awaiting requested review from developit

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@JoviDeCroock @coveralls @rschristian