Skip to content

Experiment: Remove conditional error bubbling#3234

Draft
developit wants to merge 2 commits intomainfrom
experiment-remove-conditional-error-bubbling
Draft

Experiment: Remove conditional error bubbling#3234
developit wants to merge 2 commits intomainfrom
experiment-remove-conditional-error-bubbling

Conversation

@developit
Copy link
Copy Markdown
Member

@developit developit commented Jul 23, 2021 •
edited
Loading

This fixes #3233 by making error bubbling/propagation stop at the first componentDidCatch boundary, regardless of whether calling the method happens to setState (or otherwise mark its component as dirty).

This will fail our two conditional error bubbling tests, as it changes the behavior those are verifying.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 23, 2021 •
edited
Loading

📊 Tachometer Benchmark Results

Summary

duration

  • 02_replace1k: unsure 🔍 -2% - +3% (-2.19ms - +3.50ms)
    preact-local vs preact-master
  • 03_update10th1k_x16: unsure 🔍 -5% - +2% (-1.99ms - +1.05ms)
    preact-local vs preact-master
  • 07_create10k: unsure 🔍 -2% - +1% (-21.45ms - +12.54ms)
    preact-local vs preact-master
  • filter_list: unsure 🔍 -3% - +1% (-0.92ms - +0.36ms)
    preact-local vs preact-master
  • hydrate1k: unsure 🔍 -9% - +2% (-17.55ms - +4.25ms)
    preact-local vs preact-master
  • many_updates: unsure 🔍 -5% - +3% (-1.58ms - +1.09ms)
    preact-local vs preact-master
  • text_update: unsure 🔍 -5% - +3% (-0.17ms - +0.12ms)
    preact-local vs preact-master

usedJSHeapSize

  • 02_replace1k: unsure 🔍 -0% - +0% (-0.01ms - +0.00ms)
    preact-local vs preact-master
  • 03_update10th1k_x16: unsure 🔍 -0% - +0% (-0.00ms - +0.01ms)
    preact-local vs preact-master
  • 07_create10k: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-master
  • filter_list: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-master
  • hydrate1k: unsure 🔍 -1% - +0% (-0.08ms - +0.01ms)
    preact-local vs preact-master
  • many_updates: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-master
  • text_update: unsure 🔍 -0% - -0% (-0.00ms - -0.00ms)
    preact-local vs preact-master

Results

02_replace1k

duration

VersionAvg timevs preact-mastervs preact-local
preact-master117.04ms - 121.25ms-unsure 🔍
-3% - +2%
-3.50ms - +2.19ms
preact-local117.89ms - 121.72msunsure 🔍
-2% - +3%
-2.19ms - +3.50ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master3.47ms - 3.48ms-unsure 🔍
-0% - +0%
-0.00ms - +0.01ms
preact-local3.47ms - 3.48msunsure 🔍
-0% - +0%
-0.01ms - +0.00ms		-

run-warmup-0

VersionAvg timevs preact-mastervs preact-local
preact-master44.06ms - 45.55ms-unsure 🔍
-2% - +3%
-0.79ms - +1.21ms
preact-local43.93ms - 45.26msunsure 🔍
-3% - +2%
-1.21ms - +0.79ms		-

run-warmup-1

VersionAvg timevs preact-mastervs preact-local
preact-master66.77ms - 69.44ms-unsure 🔍
-3% - +3%
-1.79ms - +1.97ms
preact-local66.69ms - 69.34msunsure 🔍
-3% - +3%
-1.97ms - +1.79ms		-

run-warmup-2

VersionAvg timevs preact-mastervs preact-local
preact-master56.98ms - 63.19ms-unsure 🔍
-4% - +11%
-2.48ms - +6.33ms
preact-local55.03ms - 61.29msunsure 🔍
-10% - +4%
-6.33ms - +2.48ms		-

run-warmup-3

VersionAvg timevs preact-mastervs preact-local
preact-master45.59ms - 52.36ms-unsure 🔍
-12% - +6%
-6.12ms - +3.21ms
preact-local47.21ms - 53.63msunsure 🔍
-7% - +13%
-3.21ms - +6.12ms		-

run-warmup-4

VersionAvg timevs preact-mastervs preact-local
preact-master64.13ms - 67.43ms-unsure 🔍
-5% - +2%
-3.39ms - +1.40ms
preact-local65.04ms - 68.50msunsure 🔍
-2% - +5%
-1.40ms - +3.39ms		-

run-final

VersionAvg timevs preact-mastervs preact-local
preact-master38.23ms - 40.97ms-unsure 🔍
-7% - +3%
-2.91ms - +1.09ms
preact-local39.06ms - 41.96msunsure 🔍
-3% - +7%
-1.09ms - +2.91ms		-
03_update10th1k_x16

duration

VersionAvg timevs preact-mastervs preact-local
preact-master41.73ms - 43.85ms-unsure 🔍
-3% - +5%
-1.05ms - +1.99ms
preact-local41.23ms - 43.41msunsure 🔍
-5% - +2%
-1.99ms - +1.05ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master3.40ms - 3.40ms-unsure 🔍
-0% - +0%
-0.01ms - +0.00ms
preact-local3.40ms - 3.40msunsure 🔍
-0% - +0%
-0.00ms - +0.01ms		-
07_create10k

duration

VersionAvg timevs preact-mastervs preact-local
preact-master1250.08ms - 1273.23ms-unsure 🔍
-1% - +2%
-12.54ms - +21.45ms
preact-local1244.75ms - 1269.64msunsure 🔍
-2% - +1%
-21.45ms - +12.54ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master25.32ms - 25.32ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-local25.32ms - 25.33msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-
filter_list

duration

VersionAvg timevs preact-mastervs preact-local
preact-master28.70ms - 29.58ms-unsure 🔍
-1% - +3%
-0.36ms - +0.92ms
preact-local28.40ms - 29.32msunsure 🔍
-3% - +1%
-0.92ms - +0.36ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master1.54ms - 1.55ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-local1.55ms - 1.55msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-
hydrate1k

duration

VersionAvg timevs preact-mastervs preact-local
preact-master192.71ms - 206.26ms-unsure 🔍
-2% - +9%
-4.25ms - +17.55ms
preact-local184.30ms - 201.37msunsure 🔍
-9% - +2%
-17.55ms - +4.25ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master6.19ms - 6.25ms-unsure 🔍
-0% - +1%
-0.01ms - +0.08ms
preact-local6.15ms - 6.22msunsure 🔍
-1% - +0%
-0.08ms - +0.01ms		-
many_updates

duration

VersionAvg timevs preact-mastervs preact-local
preact-master33.86ms - 35.78ms-unsure 🔍
-3% - +5%
-1.09ms - +1.58ms
preact-local33.65ms - 35.51msunsure 🔍
-5% - +3%
-1.58ms - +1.09ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master4.61ms - 4.61ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-local4.61ms - 4.61msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-
text_update

duration

VersionAvg timevs preact-mastervs preact-local
preact-master3.47ms - 3.67ms-unsure 🔍
-3% - +5%
-0.12ms - +0.17ms
preact-local3.43ms - 3.65msunsure 🔍
-5% - +3%
-0.17ms - +0.12ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master0.78ms - 0.78ms-unsure 🔍
+0% - +0%
+0.00ms - +0.00ms
preact-local0.78ms - 0.78msunsure 🔍
-0% - -0%
-0.00ms - -0.00ms		-

tachometer-reporter-action v2 for Benchmarks

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 23, 2021 •
edited
Loading

Size Change: -10 B (0%)

Total Size: 42.1 kB

Filename Size Change
dist/preact.js 3.96 kB -2 B (0%)
dist/preact.min.js 4 kB -2 B (0%)
dist/preact.module.js 3.99 kB -3 B (0%)
dist/preact.umd.js 4.03 kB -3 B (0%)
ℹ️ View Unchanged
Filename Size Change
compat/dist/compat.js 3.44 kB 0 B
compat/dist/compat.module.js 3.42 kB 0 B
compat/dist/compat.umd.js 3.49 kB 0 B
debug/dist/debug.js 2.99 kB 0 B
debug/dist/debug.module.js 2.98 kB 0 B
debug/dist/debug.umd.js 3.07 kB 0 B
devtools/dist/devtools.js 232 B 0 B
devtools/dist/devtools.module.js 240 B 0 B
devtools/dist/devtools.umd.js 308 B 0 B
hooks/dist/hooks.js 1.14 kB 0 B
hooks/dist/hooks.module.js 1.16 kB 0 B
hooks/dist/hooks.umd.js 1.22 kB 0 B
jsx-runtime/dist/jsxRuntime.js 317 B 0 B
jsx-runtime/dist/jsxRuntime.module.js 327 B 0 B
jsx-runtime/dist/jsxRuntime.umd.js 395 B 0 B
test-utils/dist/testUtils.js 437 B 0 B
test-utils/dist/testUtils.module.js 439 B 0 B
test-utils/dist/testUtils.umd.js 515 B 0 B

compressed-size-action

JoviDeCroock
JoviDeCroock approved these changes Dec 3, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

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

Fixed tests in #3353

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JoviDeCroock JoviDeCroock JoviDeCroock approved these changes

@andrewiggins andrewiggins Awaiting requested review from andrewiggins

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[nextjs] Preact renders 5x when error is thrown in render and cancels rendering route

2 participants

@developit @JoviDeCroock