Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor[devtools]: lazily define source for fiber based on component stacks #28351

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Feb 15, 2024

_debugSource was removed in #28265.

This PR migrates DevTools to define source for Fiber based on component stacks. This will be done lazily for inspected elements, once user clicks on the element in the tree.

DevToolsComponentStackFrame.js was just copy-pasted from the implementation in ReactComponentStackFrame.

Symbolication part is done in #28471 and stacked on this commit.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 15, 2024
@hoxyq hoxyq force-pushed the devtools/use-component-stacks-for-defining-source-for-fiber branch from 39f88cc to b3aa93f Compare February 15, 2024 22:38
@sebmarkbage
Copy link
Collaborator

Nice. Thanks for doing this!

@hoxyq hoxyq force-pushed the devtools/use-component-stacks-for-defining-source-for-fiber branch from b3aa93f to 440fc26 Compare February 16, 2024 13:30
@hoxyq
Copy link
Contributor Author

hoxyq commented Feb 16, 2024

Fixed tests.

Reflect.construct(Fake, []);
} catch (x) {
control = x;
// NOTE: keep in sync with the implementation in ReactComponentStackFrame
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This implementation was just copy-pasted from ReactComponentStackFrame.

@hoxyq hoxyq changed the title refactor[devtools]: lazily define source for fiber based on component stacks [WIP] refactor[devtools]: lazily define source for fiber based on component stacks Feb 27, 2024
@hoxyq hoxyq force-pushed the devtools/use-component-stacks-for-defining-source-for-fiber branch 3 times, most recently from 38f04cf to c9b900b Compare February 28, 2024 23:15
@hoxyq hoxyq changed the title [WIP] refactor[devtools]: lazily define source for fiber based on component stacks refactor[devtools]: lazily define source for fiber based on component stacks Feb 29, 2024
@hoxyq hoxyq requested review from EdmondChuiHW, motiz88, huntie and robhogan and removed request for sebmarkbage and rickhanlonii February 29, 2024 12:03
@hoxyq hoxyq force-pushed the devtools/use-component-stacks-for-defining-source-for-fiber branch 2 times, most recently from f8f6b78 to fe9a20a Compare March 4, 2024 23:06
@hoxyq hoxyq force-pushed the devtools/use-component-stacks-for-defining-source-for-fiber branch from fe9a20a to e450e6b Compare March 5, 2024 08:55
@hoxyq
Copy link
Contributor Author

hoxyq commented Mar 5, 2024

Reviewed by @motiz88 in #28471, so going to land this. Tested this on Chrome / FF extension, standalone shell and inline version.

@hoxyq hoxyq merged commit 61bd004 into facebook:main Mar 5, 2024
38 checks passed
@hoxyq hoxyq deleted the devtools/use-component-stacks-for-defining-source-for-fiber branch March 5, 2024 12:10
hoxyq added a commit that referenced this pull request Mar 5, 2024
Stacked on #28351, please review
only the last commit.

Top-level description of the approach:
1. Once user selects an element from the tree, frontend asks backend to
return the inspected element, this is where we simulate an error
happening in `render` function of the component and then we parse the
error stack. As an improvement, we should probably migrate from custom
implementation of error stack parser to `error-stack-parser` from npm.
2. When frontend receives the inspected element and this object is being
propagated, we create a Promise for symbolicated source, which is then
passed down to all components, which are using `source`.
3. These components use `use` hook for this promise and are wrapped in
Suspense.

Caching:
1. For browser extension, we cache Promises based on requested resource
+ key + column, also added use of
`chrome.devtools.inspectedWindow.getResource` API.
2. For standalone case (RN), we cache based on requested resource url,
we cache the content of it.
hoxyq added a commit that referenced this pull request Mar 5, 2024
* feat[devtools]: symbolicate source for inspected element
([hoxyq](https://github.com/hoxyq) in
[#28471](#28471))
* refactor[devtools]: lazily define source for fiber based on component
stacks ([hoxyq](https://github.com/hoxyq) in
[#28351](#28351))
* fix[devtools/tree/element]: onClick -> onMouseDown to handle first
click correctly ([hoxyq](https://github.com/hoxyq) in
[#28486](#28486))
* [DOM] disable legacy mode behind flag
([gnoff](https://github.com/gnoff) in
[#28468](#28468))
* Fix Broken Links In Documentation
([justindhillon](https://github.com/justindhillon) in
[#28321](#28321))
* Update /link URLs to react.dev
([rickhanlonii](https://github.com/rickhanlonii) in
[#28477](#28477))
* [tests] add support for @GATE pragma
([gnoff](https://github.com/gnoff) in
[#28479](#28479))
* Devtools: Unwrap Promise in useFormState
([eps1lon](https://github.com/eps1lon) in
[#28319](#28319))
* Add support for rendering BigInt
([eps1lon](https://github.com/eps1lon) in
[#24580](#24580))
* Include server component names in the componentStack in DEV
([sebmarkbage](https://github.com/sebmarkbage) in
[#28415](#28415))
gnoff added a commit to gnoff/next.js that referenced this pull request Mar 25, 2024
- facebook/react#28596
- facebook/react#28625
- facebook/react#28616
- facebook/react#28491
- facebook/react#28583
- facebook/react#28427
- facebook/react#28613
- facebook/react#28599
- facebook/react#28611
- facebook/react#28610
- facebook/react#28606
- facebook/react#28598
- facebook/react#28549
- facebook/react#28557
- facebook/react#28467
- facebook/react#28591
- facebook/react#28459
- facebook/react#28590
- facebook/react#28564
- facebook/react#28582
- facebook/react#28579
- facebook/react#28578
- facebook/react#28521
- facebook/react#28550
- facebook/react#28576
- facebook/react#28577
- facebook/react#28571
- facebook/react#28572
- facebook/react#28560
- facebook/react#28569
- facebook/react#28573
- facebook/react#28546
- facebook/react#28568
- facebook/react#28562
- facebook/react#28566
- facebook/react#28565
- facebook/react#28559
- facebook/react#28508
- facebook/react#20432
- facebook/react#28555
- facebook/react#24730
- facebook/react#28472
- facebook/react#27991
- facebook/react#28514
- facebook/react#28548
- facebook/react#28526
- facebook/react#28515
- facebook/react#28533
- facebook/react#28532
- facebook/react#28531
- facebook/react#28407
- facebook/react#28522
- facebook/react#28538
- facebook/react#28509
- facebook/react#28534
- facebook/react#28527
- facebook/react#28528
- facebook/react#28519
- facebook/react#28411
- facebook/react#28520
- facebook/react#28518
- facebook/react#28493
- facebook/react#28504
- facebook/react#28499
- facebook/react#28501
- facebook/react#28496
- facebook/react#28471
- facebook/react#28351
- facebook/react#28486
- facebook/react#28490
- facebook/react#28488
- facebook/react#28468
- facebook/react#28321
- facebook/react#28477
- facebook/react#28479
- facebook/react#28480
- facebook/react#28478
- facebook/react#28464
- facebook/react#28475
- facebook/react#28456
- facebook/react#28319
- facebook/react#28345
- facebook/react#28337
- facebook/react#28335
- facebook/react#28466
- facebook/react#28462
- facebook/react#28322
- facebook/react#28444
- facebook/react#28448
- facebook/react#28449
- facebook/react#28446
- facebook/react#28447
- facebook/react#24580
- facebook/react#28514
- facebook/react#28548
- facebook/react#28526
- facebook/react#28515
- facebook/react#28533
- facebook/react#28532
- facebook/react#28531
- facebook/react#28407
- facebook/react#28522
- facebook/react#28538
- facebook/react#28509
- facebook/react#28534
- facebook/react#28527
- facebook/react#28528
- facebook/react#28519
- facebook/react#28411
- facebook/react#28520
- facebook/react#28518
- facebook/react#28493
- facebook/react#28504
- facebook/react#28499
- facebook/react#28501
- facebook/react#28496
- facebook/react#28471
- facebook/react#28351
- facebook/react#28486
- facebook/react#28490
- facebook/react#28488
- facebook/react#28468
- facebook/react#28321
- facebook/react#28477
- facebook/react#28479
- facebook/react#28480
- facebook/react#28478
- facebook/react#28464
- facebook/react#28475
- facebook/react#28456
- facebook/react#28319
- facebook/react#28345
- facebook/react#28337
- facebook/react#28335
- facebook/react#28466
- facebook/react#28462
- facebook/react#28322
- facebook/react#28444
- facebook/react#28448
- facebook/react#28449
- facebook/react#28446
- facebook/react#28447
- facebook/react#24580
gnoff added a commit to gnoff/next.js that referenced this pull request Mar 25, 2024
- facebook/react#28596
- facebook/react#28625
- facebook/react#28616
- facebook/react#28491
- facebook/react#28583
- facebook/react#28427
- facebook/react#28613
- facebook/react#28599
- facebook/react#28611
- facebook/react#28610
- facebook/react#28606
- facebook/react#28598
- facebook/react#28549
- facebook/react#28557
- facebook/react#28467
- facebook/react#28591
- facebook/react#28459
- facebook/react#28590
- facebook/react#28564
- facebook/react#28582
- facebook/react#28579
- facebook/react#28578
- facebook/react#28521
- facebook/react#28550
- facebook/react#28576
- facebook/react#28577
- facebook/react#28571
- facebook/react#28572
- facebook/react#28560
- facebook/react#28569
- facebook/react#28573
- facebook/react#28546
- facebook/react#28568
- facebook/react#28562
- facebook/react#28566
- facebook/react#28565
- facebook/react#28559
- facebook/react#28508
- facebook/react#20432
- facebook/react#28555
- facebook/react#24730
- facebook/react#28472
- facebook/react#27991
- facebook/react#28514
- facebook/react#28548
- facebook/react#28526
- facebook/react#28515
- facebook/react#28533
- facebook/react#28532
- facebook/react#28531
- facebook/react#28407
- facebook/react#28522
- facebook/react#28538
- facebook/react#28509
- facebook/react#28534
- facebook/react#28527
- facebook/react#28528
- facebook/react#28519
- facebook/react#28411
- facebook/react#28520
- facebook/react#28518
- facebook/react#28493
- facebook/react#28504
- facebook/react#28499
- facebook/react#28501
- facebook/react#28496
- facebook/react#28471
- facebook/react#28351
- facebook/react#28486
- facebook/react#28490
- facebook/react#28488
- facebook/react#28468
- facebook/react#28321
- facebook/react#28477
- facebook/react#28479
- facebook/react#28480
- facebook/react#28478
- facebook/react#28464
- facebook/react#28475
- facebook/react#28456
- facebook/react#28319
- facebook/react#28345
- facebook/react#28337
- facebook/react#28335
- facebook/react#28466
- facebook/react#28462
- facebook/react#28322
- facebook/react#28444
- facebook/react#28448
- facebook/react#28449
- facebook/react#28446
- facebook/react#28447
- facebook/react#24580
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
… stacks (facebook#28351)

`_debugSource` was removed in
facebook#28265.

This PR migrates DevTools to define `source` for Fiber based on
component stacks. This will be done lazily for inspected elements, once
user clicks on the element in the tree.

`DevToolsComponentStackFrame.js` was just copy-pasted from the
implementation in `ReactComponentStackFrame`.

Symbolication part is done in
facebook#28471 and stacked on this commit.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
)

Stacked on facebook#28351, please review
only the last commit.

Top-level description of the approach:
1. Once user selects an element from the tree, frontend asks backend to
return the inspected element, this is where we simulate an error
happening in `render` function of the component and then we parse the
error stack. As an improvement, we should probably migrate from custom
implementation of error stack parser to `error-stack-parser` from npm.
2. When frontend receives the inspected element and this object is being
propagated, we create a Promise for symbolicated source, which is then
passed down to all components, which are using `source`.
3. These components use `use` hook for this promise and are wrapped in
Suspense.

Caching:
1. For browser extension, we cache Promises based on requested resource
+ key + column, also added use of
`chrome.devtools.inspectedWindow.getResource` API.
2. For standalone case (RN), we cache based on requested resource url,
we cache the content of it.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
* feat[devtools]: symbolicate source for inspected element
([hoxyq](https://github.com/hoxyq) in
[facebook#28471](facebook#28471))
* refactor[devtools]: lazily define source for fiber based on component
stacks ([hoxyq](https://github.com/hoxyq) in
[facebook#28351](facebook#28351))
* fix[devtools/tree/element]: onClick -> onMouseDown to handle first
click correctly ([hoxyq](https://github.com/hoxyq) in
[facebook#28486](facebook#28486))
* [DOM] disable legacy mode behind flag
([gnoff](https://github.com/gnoff) in
[facebook#28468](facebook#28468))
* Fix Broken Links In Documentation
([justindhillon](https://github.com/justindhillon) in
[facebook#28321](facebook#28321))
* Update /link URLs to react.dev
([rickhanlonii](https://github.com/rickhanlonii) in
[facebook#28477](facebook#28477))
* [tests] add support for @GATE pragma
([gnoff](https://github.com/gnoff) in
[facebook#28479](facebook#28479))
* Devtools: Unwrap Promise in useFormState
([eps1lon](https://github.com/eps1lon) in
[facebook#28319](facebook#28319))
* Add support for rendering BigInt
([eps1lon](https://github.com/eps1lon) in
[facebook#24580](facebook#24580))
* Include server component names in the componentStack in DEV
([sebmarkbage](https://github.com/sebmarkbage) in
[facebook#28415](facebook#28415))
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
… stacks (#28351)

`_debugSource` was removed in
#28265.

This PR migrates DevTools to define `source` for Fiber based on
component stacks. This will be done lazily for inspected elements, once
user clicks on the element in the tree.

`DevToolsComponentStackFrame.js` was just copy-pasted from the
implementation in `ReactComponentStackFrame`.

Symbolication part is done in
#28471 and stacked on this commit.

DiffTrain build for commit 61bd004.
eps1lon added a commit to vercel/next.js that referenced this pull request Apr 19, 2024
### React upstream changes

- facebook/react#28643
- facebook/react#28628
- facebook/react#28361
- facebook/react#28513
- facebook/react#28299
- facebook/react#28617
- facebook/react#28618
- facebook/react#28621
- facebook/react#28614
- facebook/react#28596
- facebook/react#28625
- facebook/react#28616
- facebook/react#28491
- facebook/react#28583
- facebook/react#28427
- facebook/react#28613
- facebook/react#28599
- facebook/react#28611
- facebook/react#28610
- facebook/react#28606
- facebook/react#28598
- facebook/react#28549
- facebook/react#28557
- facebook/react#28467
- facebook/react#28591
- facebook/react#28459
- facebook/react#28590
- facebook/react#28564
- facebook/react#28582
- facebook/react#28579
- facebook/react#28578
- facebook/react#28521
- facebook/react#28550
- facebook/react#28576
- facebook/react#28577
- facebook/react#28571
- facebook/react#28572
- facebook/react#28560
- facebook/react#28569
- facebook/react#28573
- facebook/react#28546
- facebook/react#28568
- facebook/react#28562
- facebook/react#28566
- facebook/react#28565
- facebook/react#28559
- facebook/react#28508
- facebook/react#20432
- facebook/react#28555
- facebook/react#24730
- facebook/react#28472
- facebook/react#27991
- facebook/react#28514
- facebook/react#28548
- facebook/react#28526
- facebook/react#28515
- facebook/react#28533
- facebook/react#28532
- facebook/react#28531
- facebook/react#28407
- facebook/react#28522
- facebook/react#28538
- facebook/react#28509
- facebook/react#28534
- facebook/react#28527
- facebook/react#28528
- facebook/react#28519
- facebook/react#28411
- facebook/react#28520
- facebook/react#28518
- facebook/react#28493
- facebook/react#28504
- facebook/react#28499
- facebook/react#28501
- facebook/react#28496
- facebook/react#28471
- facebook/react#28351
- facebook/react#28486
- facebook/react#28490
- facebook/react#28488
- facebook/react#28468
- facebook/react#28321
- facebook/react#28477
- facebook/react#28479
- facebook/react#28480
- facebook/react#28478
- facebook/react#28464
- facebook/react#28475
- facebook/react#28456
- facebook/react#28319
- facebook/react#28345
- facebook/react#28337
- facebook/react#28335
- facebook/react#28466
- facebook/react#28462
- facebook/react#28322
- facebook/react#28444
- facebook/react#28448
- facebook/react#28449
- facebook/react#28446
- facebook/react#28447
- facebook/react#24580
- facebook/react#28514
- facebook/react#28548
- facebook/react#28526
- facebook/react#28515
- facebook/react#28533
- facebook/react#28532
- facebook/react#28531
- facebook/react#28407
- facebook/react#28522
- facebook/react#28538
- facebook/react#28509
- facebook/react#28534
- facebook/react#28527
- facebook/react#28528
- facebook/react#28519
- facebook/react#28411
- facebook/react#28520
- facebook/react#28518
- facebook/react#28493
- facebook/react#28504
- facebook/react#28499
- facebook/react#28501
- facebook/react#28496
- facebook/react#28471
- facebook/react#28351
- facebook/react#28486
- facebook/react#28490
- facebook/react#28488
- facebook/react#28468
- facebook/react#28321
- facebook/react#28477
- facebook/react#28479
- facebook/react#28480
- facebook/react#28478
- facebook/react#28464
- facebook/react#28475
- facebook/react#28456
- facebook/react#28319
- facebook/react#28345
- facebook/react#28337
- facebook/react#28335
- facebook/react#28466
- facebook/react#28462
- facebook/react#28322
- facebook/react#28444
- facebook/react#28448
- facebook/react#28449
- facebook/react#28446
- facebook/react#28447
- facebook/react#24580
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants