Skip to content

Avoid overriding render method in Suspense#2917

Merged
marvinhagemeister merged 2 commits intomasterfrom
avoid-overriding-render-method
Jan 9, 2021
Merged

Avoid overriding render method in Suspense#2917
marvinhagemeister merged 2 commits intomasterfrom
avoid-overriding-render-method

Conversation

@andrewiggins
Copy link
Copy Markdown
Member

@andrewiggins andrewiggins commented Jan 9, 2021 •
edited
Loading

The current workaround to enable suspended components to resume without resolving their promise requires overriding the component's render method. Overriding methods on objects we don't own isn't a great pattern and while we do it for componentWillUnmount in Suspense, its something we should move away from (should really be using an options.unmount plugin).

We may come back to this use case later after a planned rewrite (more details coming soon) which will hopefully simplify many of these scenarios.

And FWIW, React doesn't support this scenario (resuming from Suspense without resolving the Promise)

@andrewiggins
Avoid overriding render method in Suspense
7a0eb32 
The current workaround to enable suspended components to resume without resolving their promise requires overriding the component's render method. Overriding methods on objects we don't own isn't a great pattern and while we do it for componentWillUnmount its something we'd like to move away from (should really be using an options.unmount plugin).

We may come back to this use case later after a planned rewrite (more details coming soon) which will hopefully simplify many of these scenarios
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 9, 2021 •
edited
Loading

📊 Tachometer Benchmark Results

Summary

duration

  • 02_replace1k: unsure 🔍 -2% - +1% (-2.73ms - +1.41ms)
    preact-local vs preact-master
  • 03_update10th1k_x16: unsure 🔍 -3% - +5% (-0.92ms - +1.47ms)
    preact-local vs preact-master
  • 07_create10k: unsure 🔍 -0% - +1% (-4.31ms - +8.42ms)
    preact-local vs preact-master
  • filter_list: unsure 🔍 -5% - +1% (-1.45ms - +0.26ms)
    preact-local vs preact-master
  • hydrate1k: unsure 🔍 -1% - +3% (-1.38ms - +5.24ms)
    preact-local vs preact-master
  • many_updates: unsure 🔍 -12% - +2% (-5.49ms - +0.91ms)
    preact-local vs preact-master
  • text_update: unsure 🔍 -1% - +5% (-0.01ms - +0.14ms)
    preact-local vs preact-master

usedJSHeapSize

  • 02_replace1k: unsure 🔍 -0% - +0% (-0.00ms - +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 🔍 -0% - +0% (-0.01ms - +0.03ms)
    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
  • Browser: chrome-headless 87.0.4280.88
  • Sample size: 80
  • Built by: CI #685

  • Commit: 0a7eeb2

duration

VersionAvg timevs preact-mastervs preact-local
preact-master176.94ms - 179.93ms-unsure 🔍
-1% - +2%
-1.41ms - +2.73ms
preact-local176.35ms - 179.21msunsure 🔍
-2% - +1%
-2.73ms - +1.41ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master3.60ms - 3.60ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-local3.60ms - 3.61msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-

run-warmup-0

VersionAvg timevs preact-mastervs preact-local
preact-master66.62ms - 67.82ms-unsure 🔍
-1% - +2%
-0.51ms - +1.16ms
preact-local66.32ms - 67.47msunsure 🔍
-2% - +1%
-1.16ms - +0.51ms		-

run-warmup-1

VersionAvg timevs preact-mastervs preact-local
preact-master102.81ms - 104.52ms-unsure 🔍
-1% - +2%
-0.75ms - +1.73ms
preact-local102.28ms - 104.07msunsure 🔍
-2% - +1%
-1.73ms - +0.75ms		-

run-warmup-2

VersionAvg timevs preact-mastervs preact-local
preact-master119.22ms - 120.92ms-unsure 🔍
-1% - +1%
-0.63ms - +1.62ms
preact-local118.84ms - 120.31msunsure 🔍
-1% - +1%
-1.62ms - +0.63ms		-

run-warmup-3

VersionAvg timevs preact-mastervs preact-local
preact-master64.70ms - 65.44ms-unsure 🔍
-1% - +1%
-0.63ms - +0.63ms
preact-local64.56ms - 65.58msunsure 🔍
-1% - +1%
-0.63ms - +0.63ms		-

run-warmup-4

VersionAvg timevs preact-mastervs preact-local
preact-master123.02ms - 128.89ms-unsure 🔍
-3% - +4%
-3.60ms - +4.74ms
preact-local122.43ms - 128.35msunsure 🔍
-4% - +3%
-4.74ms - +3.60ms		-

run-final

VersionAvg timevs preact-mastervs preact-local
preact-master63.99ms - 64.94ms-unsure 🔍
-0% - +2%
-0.17ms - +1.10ms
preact-local63.58ms - 64.42msunsure 🔍
-2% - +0%
-1.10ms - +0.17ms		-
03_update10th1k_x16
  • Browser: chrome-headless 87.0.4280.88
  • Sample size: 90
  • Built by: CI #685

  • Commit: 0a7eeb2

duration

VersionAvg timevs preact-mastervs preact-local
preact-master31.06ms - 32.62ms-unsure 🔍
-5% - +3%
-1.47ms - +0.92ms
preact-local31.21ms - 33.02msunsure 🔍
-3% - +5%
-0.92ms - +1.47ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master3.52ms - 3.53ms-unsure 🔍
-0% - +0%
-0.01ms - +0.00ms
preact-local3.52ms - 3.53msunsure 🔍
-0% - +0%
-0.00ms - +0.01ms		-
07_create10k
  • Browser: chrome-headless 87.0.4280.88
  • Sample size: 50
  • Built by: CI #685

  • Commit: 0a7eeb2

duration

VersionAvg timevs preact-mastervs preact-local
preact-master1630.37ms - 1638.19ms-unsure 🔍
-1% - +0%
-8.42ms - +4.31ms
preact-local1631.31ms - 1641.35msunsure 🔍
-0% - +1%
-4.31ms - +8.42ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master25.99ms - 25.99ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-local25.99ms - 25.99msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-
filter_list
  • Browser: chrome-headless 87.0.4280.88
  • Sample size: 90
  • Built by: CI #685

  • Commit: 0a7eeb2

duration

VersionAvg timevs preact-mastervs preact-local
preact-master30.46ms - 32.01ms-unsure 🔍
-1% - +5%
-0.26ms - +1.45ms
preact-local30.28ms - 31.00msunsure 🔍
-5% - +1%
-1.45ms - +0.26ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master1.60ms - 1.60ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-local1.60ms - 1.60msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-
hydrate1k
  • Browser: chrome-headless 87.0.4280.88
  • Sample size: 50
  • Built by: CI #685

  • Commit: 0a7eeb2

duration

VersionAvg timevs preact-mastervs preact-local
preact-master163.40ms - 166.85ms-unsure 🔍
-3% - +1%
-5.24ms - +1.38ms
preact-local164.23ms - 169.88msunsure 🔍
-1% - +3%
-1.38ms - +5.24ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master6.19ms - 6.21ms-unsure 🔍
-0% - +0%
-0.03ms - +0.01ms
preact-local6.19ms - 6.22msunsure 🔍
-0% - +0%
-0.01ms - +0.03ms		-
many_updates
  • Browser: chrome-headless 87.0.4280.88
  • Sample size: 80
  • Built by: CI #685

  • Commit: 0a7eeb2

duration

VersionAvg timevs preact-mastervs preact-local
preact-master41.50ms - 46.32ms-unsure 🔍
-2% - +13%
-0.91ms - +5.49ms
preact-local39.52ms - 43.73msunsure 🔍
-12% - +2%
-5.49ms - +0.91ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master4.85ms - 4.85ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-local4.85ms - 4.85msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms		-
text_update
  • Browser: chrome-headless 87.0.4280.88
  • Sample size: 140
  • Built by: CI #685

  • Commit: 0a7eeb2

duration

VersionAvg timevs preact-mastervs preact-local
preact-master2.80ms - 2.88ms-unsure 🔍
-5% - +0%
-0.14ms - +0.01ms
preact-local2.83ms - 2.97msunsure 🔍
-1% - +5%
-0.01ms - +0.14ms		-

usedJSHeapSize

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

tachometer-reporter-action v2 for CI

@andrewiggins andrewiggins mentioned this pull request Jan 9, 2021
Merged
@coveralls
Copy link
Copy Markdown

coveralls commented Jan 9, 2021 •
edited
Loading

Coverage Status

Coverage increased (+0.8%) to 99.615% when pulling 810699a on avoid-overriding-render-method into bd3c655 on master.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 9, 2021 •
edited
Loading

Size Change: -94 B (0%)

Total Size: 42 kB

Filename Size Change
compat/dist/compat.js 3.34 kB -30 B (0%)
compat/dist/compat.module.js 3.33 kB -35 B (1%)
compat/dist/compat.umd.js 3.39 kB -29 B (0%)
ℹ️ View Unchanged
Filename Size Change
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 185 B 0 B
devtools/dist/devtools.module.js 195 B 0 B
devtools/dist/devtools.umd.js 261 B 0 B
dist/preact.js 4.04 kB 0 B
dist/preact.min.js 4.07 kB 0 B
dist/preact.module.js 4.06 kB 0 B
dist/preact.umd.js 4.11 kB 0 B
hooks/dist/hooks.js 1.13 kB 0 B
hooks/dist/hooks.module.js 1.15 kB 0 B
hooks/dist/hooks.umd.js 1.2 kB 0 B
jsx-runtime/dist/jsxRuntime.js 327 B 0 B
jsx-runtime/dist/jsxRuntime.module.js 335 B 0 B
jsx-runtime/dist/jsxRuntime.umd.js 405 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

@marvinhagemeister marvinhagemeister merged commit 7b12506 into master Jan 9, 2021
@marvinhagemeister marvinhagemeister deleted the avoid-overriding-render-method branch January 9, 2021 21:46
@tanhauhau tanhauhau mentioned this pull request Jan 11, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marvinhagemeister marvinhagemeister marvinhagemeister approved these changes

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

@andrewiggins @coveralls @marvinhagemeister