search

preactjs / preact

⚛ī¸ Fast 3kB React alternative with the same modern API. Components & Virtual DOM.
https://preactjs.com
MIT License
36.35k stars 1.93k forks source link

fix(core): support popover boolean attribute #4393

Closed JoviDeCroock closed 1 month ago

JoviDeCroock commented 1 month ago

Fixes https://github.com/preactjs/preact/issues/4391

When doing tests it looks like setAttribute('popover', '') is the only one that results in proper behavior here. We should double check what we do in RTS for this as well

github-actions[bot] commented 1 month ago

📊 Tachometer Benchmark Results

Summary

duration

  • create10k: unsure 🔍 -1% - +1% (-12.27ms - +9.25ms)
    preact-local vs preact-main
  • filter-list: unsure 🔍 -1% - +1% (-0.20ms - +0.15ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 -3% - +1% (-2.26ms - +0.55ms)
    preact-local vs preact-main
  • many-updates: unsure 🔍 -2% - +5% (-0.24ms - +0.75ms)
    preact-local vs preact-main
  • replace1k: unsure 🔍 -1% - +3% (-0.35ms - +1.76ms)
    preact-local vs preact-main
  • text-update: unsure 🔍 -5% - +2% (-0.10ms - +0.04ms)
    preact-local vs preact-main
  • todo: unsure 🔍 -2% - +2% (-0.54ms - +0.69ms)
    preact-local vs preact-main
  • update10th1k: unsure 🔍 -3% - +2% (-0.80ms - +0.47ms)
    preact-local vs preact-main

usedJSHeapSize

  • create10k: unsure 🔍 -0% - +0% (-0.01ms - +0.01ms)
    preact-local vs preact-main
  • filter-list: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 -3% - +2% (-0.50ms - +0.30ms)
    preact-local vs preact-main
  • many-updates: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • replace1k: unsure 🔍 -0% - +1% (-0.01ms - +0.04ms)
    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.00ms - +0.00ms)
    preact-local vs preact-main

Results

create10k
  • Browser: chrome-headless
  • Sample size: 50
  • Built by: Benchmarks #1508
  • Commit: e16bb25

duration

VersionAvg timevs preact-localvs preact-main
preact-local878.94ms - 894.17ms-unsure 🔍
-1% - +1%
-12.27ms - +9.25ms
preact-main880.46ms - 895.66msunsure 🔍
-1% - +1%
-9.25ms - +12.27ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local27.30ms - 27.31ms-unsure 🔍
-0% - +0%
-0.01ms - +0.01ms
preact-main27.30ms - 27.31msunsure 🔍
-0% - +0%
-0.01ms - +0.01ms		-
filter-list
  • Browser: chrome-headless
  • Sample size: 50
  • Built by: Benchmarks #1508
  • Commit: e16bb25

duration

VersionAvg timevs preact-localvs preact-main
preact-local16.56ms - 16.82ms-unsure 🔍
-1% - +1%
-0.20ms - +0.15ms
preact-main16.60ms - 16.83msunsure 🔍
-1% - +1%
-0.15ms - +0.20ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.76ms - 1.77ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main1.76ms - 1.77msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-
hydrate1k
  • Browser: chrome-headless
  • Sample size: 50
  • Built by: Benchmarks #1508
  • Commit: e16bb25

duration

VersionAvg timevs preact-localvs preact-main
preact-local72.30ms - 72.91ms-unsure 🔍
-3% - +1%
-2.26ms - +0.55ms
preact-main72.09ms - 74.83msunsure 🔍
-1% - +3%
-0.55ms - +2.26ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local14.35ms - 14.89ms-unsure 🔍
-3% - +2%
-0.50ms - +0.30ms
preact-main14.42ms - 15.01msunsure 🔍
-2% - +3%
-0.30ms - +0.50ms		-
many-updates
  • Browser: chrome-headless
  • Sample size: 80
  • Built by: Benchmarks #1508
  • Commit: e16bb25

duration

VersionAvg timevs preact-localvs preact-main
preact-local15.00ms - 15.86ms-unsure 🔍
-2% - +5%
-0.24ms - +0.75ms
preact-main14.92ms - 15.43msunsure 🔍
-5% - +2%
-0.75ms - +0.24ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local4.95ms - 4.95ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main4.94ms - 4.95msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-
replace1k
  • Browser: chrome-headless
  • Sample size: 100
  • Built by: Benchmarks #1508
  • Commit: e16bb25

duration

VersionAvg timevs preact-localvs preact-main
preact-local69.86ms - 71.46ms-unsure 🔍
-1% - +3%
-0.35ms - +1.76ms
preact-main69.27ms - 70.64msunsure 🔍
-2% - +0%
-1.76ms - +0.35ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local3.78ms - 3.81ms-unsure 🔍
-0% - +1%
-0.01ms - +0.04ms
preact-main3.76ms - 3.80msunsure 🔍
-1% - +0%
-0.04ms - +0.01ms		-

run-warmup-0

VersionAvg timevs preact-localvs preact-main
preact-local29.36ms - 30.33ms-unsure 🔍
-0% - +4%
+0.00ms - +1.09ms
preact-main29.05ms - 29.55msfaster ✔
0% - 4%
0.00ms - 1.09ms		-

run-warmup-1

VersionAvg timevs preact-localvs preact-main
preact-local34.46ms - 36.28ms-unsure 🔍
-4% - +4%
-1.38ms - +1.25ms
preact-main34.49ms - 36.38msunsure 🔍
-4% - +4%
-1.25ms - +1.38ms		-

run-warmup-2

VersionAvg timevs preact-localvs preact-main
preact-local26.32ms - 26.85ms-unsure 🔍
-2% - +1%
-0.53ms - +0.19ms
preact-main26.52ms - 27.00msunsure 🔍
-1% - +2%
-0.19ms - +0.53ms		-

run-warmup-3

VersionAvg timevs preact-localvs preact-main
preact-local24.99ms - 25.82ms-unsure 🔍
-1% - +3%
-0.29ms - +0.86ms
preact-main24.72ms - 25.52msunsure 🔍
-3% - +1%
-0.86ms - +0.29ms		-

run-warmup-4

VersionAvg timevs preact-localvs preact-main
preact-local26.29ms - 27.57ms-faster ✔
0% - 7%
0.06ms - 1.92ms
preact-main27.24ms - 28.60msslower ❌
0% - 7%
0.06ms - 1.92ms		-

run-final

VersionAvg timevs preact-localvs preact-main
preact-local23.65ms - 24.72ms-unsure 🔍
-2% - +4%
-0.48ms - +0.93ms
preact-main23.50ms - 24.42msunsure 🔍
-4% - +2%
-0.93ms - +0.48ms		-
text-update
  • Browser: chrome-headless
  • Sample size: 150
  • Built by: Benchmarks #1508
  • Commit: e16bb25

duration

VersionAvg timevs preact-localvs preact-main
preact-local2.03ms - 2.13ms-unsure 🔍
-5% - +2%
-0.10ms - +0.04ms
preact-main2.06ms - 2.16msunsure 🔍
-2% - +5%
-0.04ms - +0.10ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local0.98ms - 0.98ms-unsure 🔍
+0% - +0%
+0.00ms - +0.00ms
preact-main0.98ms - 0.98msunsure 🔍
-0% - -0%
-0.00ms - -0.00ms		-
todo
  • Browser: chrome-headless
  • Sample size: 50
  • Built by: Benchmarks #1508
  • Commit: e16bb25

duration

VersionAvg timevs preact-localvs preact-main
preact-local29.30ms - 30.41ms-unsure 🔍
-2% - +2%
-0.54ms - +0.69ms
preact-main29.51ms - 30.06msunsure 🔍
-2% - +2%
-0.69ms - +0.54ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.25ms - 1.26ms-unsure 🔍
+0% - +0%
+0.00ms - +0.00ms
preact-main1.25ms - 1.25msunsure 🔍
-0% - -0%
-0.00ms - -0.00ms		-
update10th1k
  • Browser: chrome-headless
  • Sample size: 50
  • Built by: Benchmarks #1508
  • Commit: e16bb25

duration

VersionAvg timevs preact-localvs preact-main
preact-local29.48ms - 30.24ms-unsure 🔍
-3% - +2%
-0.80ms - +0.47ms
preact-main29.52ms - 30.54msunsure 🔍
-2% - +3%
-0.47ms - +0.80ms		-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local3.74ms - 3.74ms-unsure 🔍
+0% - +0%
+0.00ms - +0.00ms
preact-main3.74ms - 3.74msunsure 🔍
-0% - -0%
-0.00ms - -0.00ms		-

tachometer-reporter-action v2 for Benchmarks

github-actions[bot] commented 1 month ago

Size Change: +72 B (+0.12%)

Total Size: 61.5 kB

Filename Size Change
dist/preact.js 4.65 kB +12 B (+0.26%)
dist/preact.min.js 4.68 kB +12 B (+0.26%)
dist/preact.min.module.js 4.67 kB +12 B (+0.26%)
dist/preact.min.umd.js 4.71 kB +11 B (+0.23%)
dist/preact.module.js 4.67 kB +12 B (+0.26%)
dist/preact.umd.js 4.72 kB +13 B (+0.28%)
ℹī¸ View Unchanged | Filename | Size | | :--- | :---: | | `compat/dist/compat.js` | 4.09 kB | | `compat/dist/compat.module.js` | 4.01 kB | | `compat/dist/compat.umd.js` | 4.14 kB | | `debug/dist/debug.js` | 3.71 kB | | `debug/dist/debug.module.js` | 3.71 kB | | `debug/dist/debug.umd.js` | 3.79 kB | | `devtools/dist/devtools.js` | 231 B | | `devtools/dist/devtools.module.js` | 240 B | | `devtools/dist/devtools.umd.js` | 314 B | | `hooks/dist/hooks.js` | 1.55 kB | | `hooks/dist/hooks.module.js` | 1.59 kB | | `hooks/dist/hooks.umd.js` | 1.63 kB | | `jsx-runtime/dist/jsxRuntime.js` | 976 B | | `jsx-runtime/dist/jsxRuntime.module.js` | 949 B | | `jsx-runtime/dist/jsxRuntime.umd.js` | 1.06 kB | | `test-utils/dist/testUtils.js` | 453 B | | `test-utils/dist/testUtils.module.js` | 454 B | | `test-utils/dist/testUtils.umd.js` | 536 B |

compressed-size-action

coveralls commented 1 month ago

Coverage Status

coverage: 99.61%. remained the same when pulling e16bb252b5fdfce844c9d2cda797501b69016d74 on support-boolean-attribute into 85bed330f3d73ce896661a61d501c714d36e0096 on main.

rschristian commented 1 month ago

RTS looks correct, as it matches other boolean attributes for the most part.

Not sure if it's smaller/faster, but we can set the prop, but we'd need to coerce it to the right value first, as attribute reflection uses the provided prop value (whereas the DOM property will coerce the value to auto/manual internally itself -- shame attribute reflection doesn't use the result of this coercion) Oh nvm, all nullish values end up as empty strings in our props. Would need that to change.

robertknight commented 1 month ago

Thanks for the quick turnaround @JoviDeCroock!

I wonder if there is a more general way to test whether attributes behave like this?