fix(hooks): memo being too lazy with repeated renders #4426

JoviDeCroock · 2024-06-28T08:38:33Z

This now equalises our behavior with React, we need to ensure that when we repeat a render we throwaway queued effects, this is done in _render. We however want to maintain achieved hook-states for i.e. useMemo as they are synchronous anyway, React seems to achieve this by disposing the WIP but retaining hooks-state. This PR makes us behave similarly.

Fixes #4422

github-actions · 2024-06-28T08:38:46Z

📊 Tachometer Benchmark Results

Summary

duration

create10k: unsure 🔍 -1% - +0% (-7.46ms - +2.92ms)
^{preact-local vs preact-main}
filter-list: unsure 🔍 -0% - +1% (-0.08ms - +0.17ms)
^{preact-local vs preact-main}
hydrate1k: unsure 🔍 -4% - +5% (-2.58ms - +3.34ms)
^{preact-local vs preact-main}
many-updates: unsure 🔍 -1% - +3% (-0.08ms - +0.54ms)
^{preact-local vs preact-main}
replace1k: unsure 🔍 -3% - +2% (-2.01ms - +1.70ms)
^{preact-local vs preact-main}
text-update: unsure 🔍 -5% - +4% (-0.09ms - +0.08ms)
^{preact-local vs preact-main}
todo: unsure 🔍 -0% - +1% (-0.02ms - +0.36ms)
^{preact-local vs preact-main}
update10th1k: unsure 🔍 -3% - +1% (-1.10ms - +0.26ms)
^{preact-local vs preact-main}

usedJSHeapSize

create10k: unsure 🔍 -0% - +0% (-0.02ms - +0.00ms)
^{preact-local vs preact-main}
filter-list: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
^{preact-local vs preact-main}
hydrate1k: unsure 🔍 -1% - +2% (-0.17ms - +0.25ms)
^{preact-local vs preact-main}
many-updates: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
^{preact-local vs preact-main}
replace1k: unsure 🔍 -1% - +0% (-0.03ms - +0.01ms)
^{preact-local vs preact-main}
text-update: unsure 🔍 -0% - +1% (-0.00ms - +0.01ms)
^{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 #1555
Commit: 1209527

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	859.96ms - 865.01ms	-	unsure 🔍 -1% - +0% -7.46ms - +2.92ms
preact-main	860.23ms - 869.29ms	unsure 🔍 -0% - +1% -2.92ms - +7.46ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	25.15ms - 25.15ms	-	unsure 🔍 -0% - +0% -0.02ms - +0.00ms
preact-main	25.15ms - 25.17ms	unsure 🔍 -0% - +0% -0.00ms - +0.02ms	-

filter-list

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1555
Commit: 1209527

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	16.67ms - 16.87ms	-	unsure 🔍 -0% - +1% -0.08ms - +0.17ms
preact-main	16.66ms - 16.79ms	unsure 🔍 -1% - +0% -0.17ms - +0.08ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.69ms - 1.69ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms
preact-main	1.69ms - 1.69ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-

hydrate1k

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1555
Commit: 1209527

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	68.81ms - 73.16ms	-	unsure 🔍 -4% - +5% -2.58ms - +3.34ms
preact-main	68.60ms - 72.62ms	unsure 🔍 -5% - +4% -3.34ms - +2.58ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	13.14ms - 13.47ms	-	unsure 🔍 -1% - +2% -0.17ms - +0.25ms
preact-main	13.13ms - 13.40ms	unsure 🔍 -2% - +1% -0.25ms - +0.17ms	-

many-updates

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1555
Commit: 1209527

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	16.48ms - 17.08ms	-	unsure 🔍 -1% - +3% -0.08ms - +0.54ms
preact-main	16.47ms - 16.64ms	unsure 🔍 -3% - +0% -0.54ms - +0.08ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	4.61ms - 4.61ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms
preact-main	4.61ms - 4.61ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-

replace1k

Browser: chrome-headless
Sample size: 100
Built by: Benchmarks #1555
Commit: 1209527

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	76.03ms - 78.63ms	-	unsure 🔍 -3% - +2% -2.01ms - +1.70ms
preact-main	76.17ms - 78.81ms	unsure 🔍 -2% - +3% -1.70ms - +2.01ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	3.51ms - 3.54ms	-	unsure 🔍 -1% - +0% -0.03ms - +0.01ms
preact-main	3.52ms - 3.55ms	unsure 🔍 -0% - +1% -0.01ms - +0.03ms	-

run-warmup-0

Version	Avg time	vs preact-local	vs preact-main
preact-local	29.30ms - 29.88ms	-	unsure 🔍 -0% - +3% -0.05ms - +0.79ms
preact-main	28.92ms - 29.52ms	unsure 🔍 -3% - +0% -0.79ms - +0.05ms	-

run-warmup-1

Version	Avg time	vs preact-local	vs preact-main
preact-local	34.93ms - 36.83ms	-	unsure 🔍 -1% - +7% -0.37ms - +2.33ms
preact-main	33.94ms - 35.86ms	unsure 🔍 -6% - +1% -2.33ms - +0.37ms	-

run-warmup-2

Version	Avg time	vs preact-local	vs preact-main
preact-local	25.63ms - 26.23ms	-	unsure 🔍 -1% - +2% -0.28ms - +0.48ms
preact-main	25.60ms - 26.07ms	unsure 🔍 -2% - +1% -0.48ms - +0.28ms	-

run-warmup-3

Version	Avg time	vs preact-local	vs preact-main
preact-local	30.01ms - 31.58ms	-	unsure 🔍 -5% - +2% -1.68ms - +0.49ms
preact-main	30.65ms - 32.14ms	unsure 🔍 -2% - +5% -0.49ms - +1.68ms	-

run-warmup-4

Version	Avg time	vs preact-local	vs preact-main
preact-local	23.04ms - 24.79ms	-	unsure 🔍 -3% - +7% -0.73ms - +1.72ms
preact-main	22.56ms - 24.27ms	unsure 🔍 -7% - +3% -1.72ms - +0.73ms	-

run-final

Version	Avg time	vs preact-local	vs preact-main
preact-local	23.59ms - 24.60ms	-	unsure 🔍 -4% - +2% -0.88ms - +0.59ms
preact-main	23.71ms - 24.77ms	unsure 🔍 -2% - +4% -0.59ms - +0.88ms	-

text-update

Browser: chrome-headless
Sample size: 120
Built by: Benchmarks #1555
Commit: 1209527

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.98ms - 2.12ms	-	unsure 🔍 -5% - +4% -0.09ms - +0.08ms
preact-main	2.00ms - 2.11ms	unsure 🔍 -4% - +5% -0.08ms - +0.09ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	0.98ms - 0.99ms	-	unsure 🔍 -0% - +1% -0.00ms - +0.01ms
preact-main	0.98ms - 0.98ms	unsure 🔍 -1% - +0% -0.01ms - +0.00ms	-

todo

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1555
Commit: 1209527

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	27.29ms - 27.58ms	-	unsure 🔍 -0% - +1% -0.02ms - +0.36ms
preact-main	27.14ms - 27.39ms	unsure 🔍 -1% - +0% -0.36ms - +0.02ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.25ms - 1.25ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms
preact-main	1.25ms - 1.25ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-

update10th1k

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1555
Commit: 1209527

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	35.17ms - 35.89ms	-	unsure 🔍 -3% - +1% -1.10ms - +0.26ms
preact-main	35.38ms - 36.53ms	unsure 🔍 -1% - +3% -0.26ms - +1.10ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	3.55ms - 3.55ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-main	3.55ms - 3.55ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

_{tachometer-reporter-action v2 for Benchmarks}

JoviDeCroock · 2024-06-28T08:39:01Z

hooks/test/browser/useMemo.test.js

 		expect(calls).to.deep.equal([
 			'doing memo',
 			'render hi',
 			'doing memo',
-			'render bye', // We expect a missing "doing memo"  here because we return to the previous args value


Justification for change can be found here

github-actions · 2024-06-28T08:39:36Z

Size Change: -88 B (-0.14%)

Total Size: 61.7 kB

Filename	Size	Change
`hooks/dist/hooks.js`	1.53 kB	-28 B (-1.8%)
`hooks/dist/hooks.module.js`	1.56 kB	-30 B (-1.88%)
`hooks/dist/hooks.umd.js`	1.6 kB	-30 B (-1.84%)

ℹ️ View Unchanged

Filename	Size
`compat/dist/compat.js`	4.1 kB
`compat/dist/compat.module.js`	4.03 kB
`compat/dist/compat.umd.js`	4.17 kB
`debug/dist/debug.js`	3.7 kB
`debug/dist/debug.module.js`	3.71 kB
`debug/dist/debug.umd.js`	3.78 kB
`devtools/dist/devtools.js`	259 B
`devtools/dist/devtools.module.js`	273 B
`devtools/dist/devtools.umd.js`	345 B
`dist/preact.js`	4.67 kB
`dist/preact.min.js`	4.7 kB
`dist/preact.min.module.js`	4.7 kB
`dist/preact.min.umd.js`	4.73 kB
`dist/preact.module.js`	4.7 kB
`dist/preact.umd.js`	4.75 kB
`jsx-runtime/dist/jsxRuntime.js`	981 B
`jsx-runtime/dist/jsxRuntime.module.js`	956 B
`jsx-runtime/dist/jsxRuntime.umd.js`	1.06 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}

hooks/test/browser/useMemo.test.js

JoviDeCroock · 2024-06-28T08:40:09Z

hooks/test/browser/combinations.test.js

@@ -477,4 +477,44 @@ describe('combinations', () => {
 			'anchor effect'
 		]);
 	});
+
+	it('should not loop infinitely', () => {


How react behaves https://codesandbox.io/p/sandbox/boring-wilson-9p6hyj?file=%2Fsrc%2Findex.js%3A12%2C16

coveralls · 2024-06-28T08:40:57Z

coverage: 99.611%. remained the same
when pulling 0068260 on memo-fix
into eb37677 on main.

coveralls · 2024-06-28T08:40:59Z

coverage: 99.611%. remained the same
when pulling d0daefa on memo-fix
into eb37677 on main.

coveralls · 2024-06-28T08:42:06Z

coverage: 99.611%. remained the same
when pulling 0068260 on memo-fix
into eb37677 on main.

coveralls · 2024-06-28T08:42:20Z

coverage: 99.611%. remained the same
when pulling 0068260 on memo-fix
into eb37677 on main.

coveralls · 2024-06-28T08:45:50Z

coverage: 99.611%. remained the same
when pulling 1209527 on memo-fix
into eb37677 on main.

coveralls · 2024-06-28T08:46:11Z

coverage: 99.611%. remained the same
when pulling 1209527 on memo-fix
into eb37677 on main.

coveralls · 2024-06-28T08:46:37Z

coverage: 99.611%. remained the same
when pulling 1209527 on memo-fix
into eb37677 on main.

coveralls · 2024-06-28T08:46:46Z

coverage: 99.611%. remained the same
when pulling 1209527 on memo-fix
into eb37677 on main.

JoviDeCroock commented Jun 28, 2024

View reviewed changes

hooks/test/browser/useMemo.test.js Outdated Show resolved Hide resolved

JoviDeCroock commented Jun 28, 2024

View reviewed changes

JoviDeCroock requested review from developit, andrewiggins and marvinhagemeister June 28, 2024 08:40

JoviDeCroock added 2 commits June 28, 2024 10:44

fix memo doing the wrong thing

74c9537

remove _pendingValue

1209527

JoviDeCroock force-pushed the memo-fix branch from 9158e3c to 1209527 Compare June 28, 2024 08:44

marvinhagemeister approved these changes Jun 28, 2024

View reviewed changes

JoviDeCroock merged commit 76f5d66 into main Jun 28, 2024
13 checks passed

JoviDeCroock deleted the memo-fix branch June 28, 2024 11:24

JoviDeCroock mentioned this pull request Jun 30, 2024

10.22.1 #4428

Merged

X-oss-byte mentioned this pull request Jul 22, 2024

[Snyk] Upgrade preact from 10.11.3 to 10.22.1 X-oss-byte/next-auth#107

Open

lorenzoAtBEIS mentioned this pull request Aug 10, 2024

[Snyk] Upgrade preact from 10.22.0 to 10.22.1 UKGovernmentBEIS/inspect_ai#228

Merged

ZeroXClem mentioned this pull request Aug 12, 2024

[Snyk] Upgrade preact from 10.20.1 to 10.22.1 ZeroXClem/metamesa#4

Closed

This was referenced Aug 14, 2024

[Snyk] Upgrade preact from 10.11.3 to 10.23.0 X-oss-byte/next-auth#109

Open

[Snyk] Upgrade preact from 10.11.3 to 10.23.1 X-oss-byte/next-auth#110

Open

ramzimalhas mentioned this pull request Aug 22, 2024

[Snyk] Upgrade preact from 10.11.3 to 10.23.1 ramzimalhas/chatgpt-advanced#2

Open

X-oss-byte mentioned this pull request Sep 3, 2024

[Snyk] Upgrade preact from 10.11.3 to 10.23.2 X-oss-byte/next-auth#113

Open

X-oss-byte mentioned this pull request Sep 8, 2024

[Snyk] Upgrade: cookie, preact, preact-render-to-string X-oss-byte/next-auth#115

Open

arkid15r mentioned this pull request Sep 11, 2024

[Snyk] Upgrade: , dayjs, marked-emoji, preact, qs, react-router-dom, sass VictoriaMetrics/VictoriaMetrics#6982

Closed

ntavassoli-dot mentioned this pull request Sep 13, 2024

[Snyk] Upgrade preact from 10.13.2 to 10.23.2 ntavassoli-dot/InstagramUnfollowers#1

Open

organich mentioned this pull request Sep 24, 2024

[Snyk] Upgrade preact from 10.19.3 to 10.23.2 organich/vscode-js-debug#3

Open

X-oss-byte mentioned this pull request Oct 5, 2024

[Snyk] Upgrade preact from 10.11.3 to 10.24.0 X-oss-byte/next-auth#123

Open

X-oss-byte mentioned this pull request Oct 18, 2024

[Snyk] Upgrade preact from 10.11.3 to 10.24.1 X-oss-byte/next-auth#125

Open

X-oss-byte mentioned this pull request Oct 25, 2024

[Snyk] Upgrade preact from 10.11.3 to 10.24.2 X-oss-byte/next-auth#127

Open

Xcaciv mentioned this pull request Oct 27, 2024

[Snyk] Upgrade preact from 10.22.0 to 10.24.2 Xcaciv/appsec-test-2024#3

Open

dwhiteGUK mentioned this pull request Oct 28, 2024

[Snyk] Upgrade preact from 10.11.3 to 10.24.2 dwhiteGUK/dlw.dev#39

Closed

X-oss-byte mentioned this pull request Nov 8, 2024

[Snyk] Upgrade preact from 10.11.3 to 10.24.3 X-oss-byte/next-auth#130

Open

fix(hooks): memo being too lazy with repeated renders #4426

fix(hooks): memo being too lazy with repeated renders #4426

Conversation

JoviDeCroock commented Jun 28, 2024 • edited Loading

github-actions bot commented Jun 28, 2024 • edited Loading

📊 Tachometer Benchmark Results

Summary

duration

usedJSHeapSize

Results

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

run-warmup-0

run-warmup-1

run-warmup-2

run-warmup-3

run-warmup-4

run-final

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

JoviDeCroock Jun 28, 2024

Choose a reason for hiding this comment

github-actions bot commented Jun 28, 2024 • edited Loading

JoviDeCroock Jun 28, 2024

Choose a reason for hiding this comment

coveralls commented Jun 28, 2024

coveralls commented Jun 28, 2024

coveralls commented Jun 28, 2024

coveralls commented Jun 28, 2024

coveralls commented Jun 28, 2024

coveralls commented Jun 28, 2024

coveralls commented Jun 28, 2024

coveralls commented Jun 28, 2024

JoviDeCroock commented Jun 28, 2024 •

edited

Loading

github-actions bot commented Jun 28, 2024 •

edited

Loading

github-actions bot commented Jun 28, 2024 •

edited

Loading