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

fix: devtools source field disappears after component remount #27297

Merged
merged 5 commits into from
Aug 29, 2023
Merged

fix: devtools source field disappears after component remount #27297

merged 5 commits into from
Aug 29, 2023

Conversation

idango10
Copy link
Contributor

Summary

Fixes: #27296

On actions that cause a component to change its signature, and therefore to remount, the _debugSource property of the fiber updates in delay and causes the devtools source field to vanish.

This issue happens in https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberBeginWork.js

function beginWork(
  current: Fiber | null,
  workInProgress: Fiber,
  renderLanes: Lanes,
): Fiber | null {
  if (__DEV__) {
    if (workInProgress._debugNeedsRemount && current !== null) {
      // This will restart the begin phase with a new fiber.
      return remountFiber(
        current,
        workInProgress,
        createFiberFromTypeAndProps(
          workInProgress.type,
          workInProgress.key,
          workInProgress.pendingProps,
          workInProgress._debugOwner || null,
          workInProgress.mode,
          workInProgress.lanes,
        ),
      );
    }
  }

  // ...

remountFiber uses the 3rd parameter as the new fiber (createFiberFromTypeAndProps(...)), but this parameter doesn’t contain a _debugSource.

How did you test this change?

Tested by monkey patching ./node_modules/react-dom/cjs/react-dom.development.js:
image

Screen.Recording.2023-08-28.at.20.39.54.2.mov

@react-sizebot
Copy link

react-sizebot commented Aug 28, 2023

Comparing: 2f36872...ef1d7e7

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js +0.01% 165.61 kB 165.63 kB = 51.88 kB 51.88 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.01% 174.69 kB 174.70 kB = 54.61 kB 54.61 kB
facebook-www/ReactDOM-prod.classic.js = 570.39 kB 570.44 kB = 100.44 kB 100.45 kB
facebook-www/ReactDOM-prod.modern.js = 554.19 kB 554.21 kB = 97.61 kB 97.61 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against ef1d7e7

Copy link
Contributor

@hoxyq hoxyq left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@hoxyq hoxyq merged commit eaa6968 into facebook:main Aug 29, 2023
2 checks passed
github-actions bot pushed a commit that referenced this pull request Aug 29, 2023
## Summary

Fixes: #27296

On actions that cause a component to change its signature, and therefore
to remount, the `_debugSource` property of the fiber updates in delay
and causes the `devtools` source field to vanish.

This issue happens in
https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberBeginWork.js

```js
function beginWork(
  current: Fiber | null,
  workInProgress: Fiber,
  renderLanes: Lanes,
): Fiber | null {
  if (__DEV__) {
    if (workInProgress._debugNeedsRemount && current !== null) {
      // This will restart the begin phase with a new fiber.
      return remountFiber(
        current,
        workInProgress,
        createFiberFromTypeAndProps(
          workInProgress.type,
          workInProgress.key,
          workInProgress.pendingProps,
          workInProgress._debugOwner || null,
          workInProgress.mode,
          workInProgress.lanes,
        ),
      );
    }
  }

  // ...
```

`remountFiber` uses the 3rd parameter as the new fiber
(`createFiberFromTypeAndProps(...)`), but this parameter doesn’t contain
a `_debugSource`.

## How did you test this change?

Tested by monkey patching
`./node_modules/react-dom/cjs/react-dom.development.js`:
<img width="1749" alt="image"
src="https://github.com/facebook/react/assets/75563024/ccaf7fab-4cc9-4c05-a48b-64db6f55dc23">

https://github.com/facebook/react/assets/75563024/0650ae5c-b277-44d1-acbb-a08d98bd38e0

DiffTrain build for [eaa6968](eaa6968)
@idango10 idango10 deleted the fix-debug-source-becomes-null-on-remount branch August 29, 2023 15:55
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…ok#27297)

## Summary

Fixes: facebook#27296

On actions that cause a component to change its signature, and therefore
to remount, the `_debugSource` property of the fiber updates in delay
and causes the `devtools` source field to vanish.

This issue happens in
https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberBeginWork.js

```js
function beginWork(
  current: Fiber | null,
  workInProgress: Fiber,
  renderLanes: Lanes,
): Fiber | null {
  if (__DEV__) {
    if (workInProgress._debugNeedsRemount && current !== null) {
      // This will restart the begin phase with a new fiber.
      return remountFiber(
        current,
        workInProgress,
        createFiberFromTypeAndProps(
          workInProgress.type,
          workInProgress.key,
          workInProgress.pendingProps,
          workInProgress._debugOwner || null,
          workInProgress.mode,
          workInProgress.lanes,
        ),
      );
    }
  }

  // ...
```

`remountFiber` uses the 3rd parameter as the new fiber
(`createFiberFromTypeAndProps(...)`), but this parameter doesn’t contain
a `_debugSource`.

## How did you test this change?

Tested by monkey patching
`./node_modules/react-dom/cjs/react-dom.development.js`:
<img width="1749" alt="image"
src="https://github.com/facebook/react/assets/75563024/ccaf7fab-4cc9-4c05-a48b-64db6f55dc23">


https://github.com/facebook/react/assets/75563024/0650ae5c-b277-44d1-acbb-a08d98bd38e0
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
## Summary

Fixes: #27296

On actions that cause a component to change its signature, and therefore
to remount, the `_debugSource` property of the fiber updates in delay
and causes the `devtools` source field to vanish.

This issue happens in
https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberBeginWork.js

```js
function beginWork(
  current: Fiber | null,
  workInProgress: Fiber,
  renderLanes: Lanes,
): Fiber | null {
  if (__DEV__) {
    if (workInProgress._debugNeedsRemount && current !== null) {
      // This will restart the begin phase with a new fiber.
      return remountFiber(
        current,
        workInProgress,
        createFiberFromTypeAndProps(
          workInProgress.type,
          workInProgress.key,
          workInProgress.pendingProps,
          workInProgress._debugOwner || null,
          workInProgress.mode,
          workInProgress.lanes,
        ),
      );
    }
  }

  // ...
```

`remountFiber` uses the 3rd parameter as the new fiber
(`createFiberFromTypeAndProps(...)`), but this parameter doesn’t contain
a `_debugSource`.

## How did you test this change?

Tested by monkey patching
`./node_modules/react-dom/cjs/react-dom.development.js`:
<img width="1749" alt="image"
src="https://github.com/facebook/react/assets/75563024/ccaf7fab-4cc9-4c05-a48b-64db6f55dc23">

https://github.com/facebook/react/assets/75563024/0650ae5c-b277-44d1-acbb-a08d98bd38e0

DiffTrain build for commit eaa6968.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: devtools source field disappears after component remount
4 participants