Add support for componentDidCatch Component method by adamsilverstein · Pull Request #886 · preactjs/preact

adamsilverstein · 2017-09-20T13:07:05Z

Amends #819 removing these changes - tests still pass.

…tors

…-support

…update minification to rename this new property to __a)

coveralls · 2017-09-20T13:11:40Z

Coverage remained the same at 100.0% when pulling b616cc8 on adamsilverstein:819 into f7834ec on developit:master.

coveralls · 2017-09-20T13:20:30Z

Coverage remained the same at 100.0% when pulling 6f6eeeb on adamsilverstein:819 into eeec8d6 on developit:master.

developit · 2018-02-23T02:48:54Z

Do we know what the size hit is here?

awinogradov · 2018-02-27T11:01:54Z

Really want it right now! Can't wait!)

awinogradov · 2018-02-27T11:34:29Z

@adamsilverstein @developit do you need help here maybe? Any timings for merge?)

ForsakenHarmony · 2018-02-27T11:37:03Z

Size comparison, possibly perf impact

developit · 2018-03-07T15:20:36Z

Looks like the size impact is an added 107 bytes. For the utility, I think we can call that justifiable.

Still need to run perf tests against this, hopefully the try blocks don't cause deopts.

To-Do: (help wanted for any of these!)

Resolve TS merge conflicts
Run local perf tests against master
Run against Preact 8.2.7 on js-framework-benchmark

marvinhagemeister · 2018-03-08T09:04:12Z

Fixed the TS conflicts 👍

awinogradov · 2018-03-14T14:02:46Z

@marvinhagemeister looks like not ;)

marvinhagemeister · 2018-03-14T14:17:21Z

@awinogradov I did, but we merged other PRs that changed some things in preact.d.ts ;)

andrewiggins · 2018-03-18T02:09:41Z

I've setup the ability to run and compare a custom build of preact in my js-frameworks-benchmark fork in the preact-componentDidCatch-keyed branch. Instructions to set it up are in the Readme of that branch.

Below are the results from running the tests with --count 3 on my laptop. However, my laptop is a work laptop I have to run these tests in a virtual machine so it may not be the best for consistent perf testing.

The startup and memory allocations were basically the same.

Duration in milliseconds ± standard deviation (Slowdown = Duration / Fastest)

Name	preact-v8.2.7-keyed	preact-componentDidCatch-keyed
create rows Duration for creating 1000 rows after the page loaded.	241.0±53.9 (1.0)	265.7±53.9 (1.1)
replace all rows Duration for updating all 1000 rows of the table (with 5 warmup iterations).	176.0±7.8 (1.0)	204.3±30.7 (1.2)
partial update Time to update the text of every 10th row (with 5 warmup iterations) for a table with 10k rows.	154.0±37.2 (1.2)	132.3±9.7 (1.0)
select row Duration to highlight a row in response to a click on the row. (with 5 warmup iterations).	5.3±1.9 (1.0)	11.3±4.9 (1.0)
swap rows Time to swap 2 rows on a 1K table. (with 5 warmup iterations).	22.7±9.5 (1.0)	26.3±3.9 (1.2)
remove row Duration to remove a row. (with 5 warmup iterations).	58.3±3.3 (1.0)	63.7±8.6 (1.1)
create many rows Duration to create 10,000 rows	2,153.0±26.8 (1.0)	2,711.0±678.7 (1.3)
append rows to large table Duration for adding 1000 rows on a table of 10,000 rows.	352.3±51.0 (1.0)	386.7±33.1 (1.1)
clear rows Duration to clear the table filled with 10.000 rows.	299.7±23.2 (1.0)	309.0±36.5 (1.0)
slowdown geometric mean	1.02	1.10

EDIT: grammar

andrewiggins · 2018-03-19T01:09:20Z

Here are some perf run results on a more reliable and powerful machine with sligtly more consistent standard deviations: https://gist.github.com/andrewiggins/19c2a1932a2540d343076434919c541b

awinogradov · 2018-03-19T11:39:17Z

So, @developit are we ready? ;)

awinogradov · 2018-03-21T14:37:13Z

Guys?) Any progress here?

ForsakenHarmony · 2018-03-21T15:56:23Z

We'll probably get a release with all the recent ts fixes out and then merge this

# Conflicts: # config/properties.json # package.json # src/vdom/component-recycler.js

awinogradov · 2018-03-30T11:01:52Z

Still waiting...

hassanbazzi · 2018-05-14T06:24:55Z

I will check up on this with the team today. Sorry y'all. There are some conflicts though with the most recent fixes that went in.

adamsilverstein · 2018-05-15T13:26:53Z

Closing in favor of #819 now that activity has resumed there :)

rpetrich and others added 8 commits August 19, 2017 23:45

Add support for Component.componentDidCatch variant with single argument

03d1f18

Allow rethrowing new errors inside componentDidCatch

7f7aeff

Merge branch 'master' into componentDidCatch-support

a036c19

Fix lint errors in test/browser/lifecycle.js

10bd841

Add test to verify that errors are caught through non-component ances…

38f943e

…tors

Merge branch 'master' into componentDidCatch-support

7f72326

Merge remote-tracking branch 'upstream/master' into componentDidCatch…

f3afbeb

…-support

Migrate ancestor component to a new _ancestorComponent property (and …

b616cc8

…update minification to rename this new property to __a)

adamsilverstein mentioned this pull request Sep 20, 2017

Add support for componentDidCatch Component method #819

Closed

revert unrelated changes in 819

d42d2eb

developit mentioned this pull request Dec 25, 2017

Implement something like error boundaries preactjs/preact-compat#453

Closed

Merge branch 'master' into 819

5fcedd7

developit added enhancement needs-perf-review labels Feb 23, 2018

developit added the help wanted label Mar 7, 2018

developit added this to the 9.0 milestone Mar 7, 2018

developit mentioned this pull request Mar 7, 2018

fix failing render side-effects #1018

Closed

Merge branch 'master' into 819

fbcd870

Merge branch 'master' into 819

e3dde82

adamsilverstein and others added 6 commits March 24, 2018 09:42

Merge branch '819' of github.com:adamsilverstein/preact into 819

7c6e4ff

Merge branch 'master' into 819

f6dcd50

Merge branch 'master' of https://github.com/developit/preact

c22fcc6

Merge branch '819' of github.com:adamsilverstein/preact into 819

b8f032a

Merge branch 'master' into 819

0407483

# Conflicts: # config/properties.json # package.json # src/vdom/component-recycler.js

remove whitespace change

6f6eeeb

adamsilverstein closed this May 15, 2018

Uh oh!

Conversation

adamsilverstein commented Sep 20, 2017

Uh oh!

coveralls commented Sep 20, 2017

Uh oh!

coveralls commented Sep 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

developit commented Feb 23, 2018

Uh oh!

awinogradov commented Feb 27, 2018

Uh oh!

awinogradov commented Feb 27, 2018

Uh oh!

ForsakenHarmony commented Feb 27, 2018

Uh oh!

developit commented Mar 7, 2018 • edited by ForsakenHarmony Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To-Do: (help wanted for any of these!)

Uh oh!

marvinhagemeister commented Mar 8, 2018

Uh oh!

awinogradov commented Mar 14, 2018

Uh oh!

marvinhagemeister commented Mar 14, 2018

Uh oh!

andrewiggins commented Mar 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Duration in milliseconds ± standard deviation (Slowdown = Duration / Fastest)

Uh oh!

andrewiggins commented Mar 19, 2018

Uh oh!

awinogradov commented Mar 19, 2018

Uh oh!

awinogradov commented Mar 21, 2018

Uh oh!

ForsakenHarmony commented Mar 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

awinogradov commented Mar 30, 2018

Uh oh!

hassanbazzi commented May 14, 2018

Uh oh!

adamsilverstein commented May 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

coveralls commented Sep 20, 2017 •

edited

Loading

developit commented Mar 7, 2018 •

edited by ForsakenHarmony

Loading

andrewiggins commented Mar 18, 2018 •

edited

Loading

ForsakenHarmony commented Mar 21, 2018 •

edited

Loading