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

Refactored backend renderer to remove most of the recursion #16627

Closed

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Aug 30, 2019

DevTools doesn't currently handle extremely deep trees (#16491) or extremely wide trees (#16501) very well, due to recursion in the backend interface. This PR removes most of that.

As a sanity test, I confirmed that after this refactor, DevTools was able to handle the following:

const Child = ({ children = null }) => children;

const Deep = () => {
  let children = null;
  for (let i = 0; i < 15000; i++) {
    children = <Child>{children}</Child>;
  }
  return children;
};

const Wide = () => {
  let children = [];
  for (let i = 0; i < 15000; i++) {
    children.push(<Child key={i} />);
  }
  return children;
};

Migrated from bvaughn/react-devtools-experimental#385

@sizebot
Copy link

sizebot commented Aug 30, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS against b8e1f81

}
}
}

// Returns whether closest unfiltered fiber parent needs to reset its child list.
//
// TODO Refactor this method to be iterative as well
function updateFiberRecursively(
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 method needs to be migrated too, but it's not going to be as straight forward as the other two since it depends on its own recursive return value.

@necolas necolas added the React Core Team Opened by a member of the React Core Team label Jan 8, 2020
@bvaughn
Copy link
Contributor Author

bvaughn commented Jan 23, 2020

We may want to do this eventually but I'm going to close for now.

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.

4 participants