Inject react-art renderer into react-devtools #13173
Conversation
Details of bundled changes.Comparing: 2a2ef7e...78e092b react-art
Generated by 🚫 dangerJS |
yunchancho
force-pushed
the
feature/react-art/inejct-devtools
branch
from
14b5575 to
a4b4b62
Compare
packages/react-art/src/ReactART.js
Outdated
| @@ -131,6 +133,13 @@ class Text extends React.Component { | |||
| } | |||
| } | |||
|
|
|||
| ARTRenderer.injectIntoDevTools({ | |||
| findFiberByHostInstance: ReactARTComponentTree.getClosestInstanceFromNode, | |||
There was a problem hiding this comment.
Does this work better than just () => null?
My impression is we only use this feature in DevTools to highlight components from DOM nodes. But ART's host instances are not DOM nodes. So we'd never make use of this.
Am I wrong? Is there any situation where ART actually does use DOM nodes (e.g. SVG mode?) In that case, can we make this code work by actually finding the fiber for an SVG node?
There was a problem hiding this comment.
@gaearon Thanks for your review. If an user sets art mode to SVG, React ART renderer creates DOM nodes for SVG. So I think that 'ref' attribute of React ART's component(e.g. Surface, Group) needs to be presented on react-devtools for debugging. The lines that you said above exists for this. In other words, that lines make react-devtools find the fiber for an DOM node of SVG.
There was a problem hiding this comment.
I still don't understand how this works. Can you explain?
instance in your code is not going to be a DOM node. It's going to be an ART object. So your getClosestInstanceFromNode implementation will give an ART object to DevTools. DevTools doesn't know what to do with those — it expects DOM nodes.
Can you explain how this implementation is useful?
There was a problem hiding this comment.
@gaearon I was a little misunderstanding about code line above . As you said, any instance from ReactART is not DOM node. I will remove ReactARTComponentTree.js file and change the line to findFiberByHostInstance: () => null.
yunchancho
force-pushed
the
feature/react-art/inejct-devtools
branch
from
a4b4b62 to
bd1bb90
Compare
This commit makes react-art renderer to be injected to react-devtools, so that component tree of the renderer is presented on debug panel of browser.
yunchancho
force-pushed
the
feature/react-art/inejct-devtools
branch
from
bd1bb90 to
5415895
Compare
|
@gaearon I have updated this commit to remove unnecessary code. |
Now a days, I am creating something using react-art.
However, I have suffered difficulties to debug my components based on react-art.
Because current react-art renderer isn't injected to react-devtools.
So I have made react-art be injected to devtools, and checked it works well in the browsers.
Could you review this my PR?
And let me know if I should do something for this PR to be accepted.