-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Conversation
Hey, I’m sorry we’re not actively reviewing PRs on this repo (or anywhere, really). We’re in churn mode trying to ship React 16 (announcement, remaining work to do). We’ll get back to reviewing stuff once 16 is shipped. That said this PR looks really great. But I think we should allow multiple renderers to inject themselves. What do you think about adding a string to |
May be the best possible solution, because it minimizes code-guessing:
I'll be back with this. Thanks for reaching out! Good luck with shipping the 16. |
Happy to take PRs to React repo adding |
22bb668
to
5d06481
Compare
Hey! I made the PR to React and updated the branch here. Tests that fail on my branch with
fail on |
Can you figure out why it is failing? |
Yeah, I'd love to. Will do it in few days. |
The errors were provided by 3rd party packages. After reinstalling all the packages in two weeks tests are alright:) |
I think the last remaining blocker for me is facebook/create-react-app#3120. |
Hi. Thanks for the comment and attention, sorry for the delay. I'm glad @tharakawj is already working on this. I'll look into ReactART's injection. |
# Conflicts: # backend/installGlobalHook.js
Small note on merging master Since we now have I assume it's the best solution before we could report table of all the checks about each renderer in popup. |
React DevTools has been rewritten and recently launched a new version 4 UI 🎉 The source code for this rewrite was done in a separate repository and is being merged into the main React repo. Thank you for taking the time to contribute to this extension 🙇♂️ Because this PR is for the version code base, I am closing it. If you'd like to contribute to the new extension, please visit the new repository. |
Resolves #700.
Most popular renderers that affect chrome, except one, do not inject themselves in DevTools hook as I stated in this comment.
We want to:
Detect multiple copies of ReactDOM
On renderer injection we:
Filtering relies on specifics of production builds of ReactDOM Stack v15.*, v0.14.* and v0.13.* and Fiber.
Issues:
react-dom
, so if you see a better way, it's cool.isReactDOM()
works similar todetectReactBuildType()
=> mirrors it's need for higher fault tolerance.Examples:
| Detected |
Filtered out |
|---|---|
| | |
| Website uses two copies of React: from vendors.js and chatlio.min.80ec1f7e.js. | Website uses two renderers: unminified ReactDOM and unminified ReactThreeRenderer (which is filtered out) |
| Result: duplicate reported. | Result: unfinified reported. |