[Electron] Support click Show Source to open file in editor#588
Conversation
suchipi
left a comment
suchipi
left a comment
There was a problem hiding this comment.
Looks good; makes sense.
| port?: number, | ||
| resolveRNStyle?: (style: number) => ?Object, | ||
| isAppActive?: () => boolean, | ||
| openFileInEditor?: () => {}, |
There was a problem hiding this comment.
This type doesn't look right; openFileInEditor has no return value. It should probably be () => void instead.
|
|
||
| import type Bridge from '../../agent/Bridge'; | ||
|
|
||
| module.exports = function setupRNOpenFile(bridge: Bridge, openFileInEditor: () => {}) { |
There was a problem hiding this comment.
The type should be () => void here, too.
jhen0409
force-pushed
the
rn-show-source
branch
from
78d5fd1 to
7faf5cc
Compare
| ) { | ||
| return; | ||
| } | ||
| var source = el._reactInternalInstance._currentElement._source; |
There was a problem hiding this comment.
This code is coupled to React internal representation (which is changing as we speak). Can you rewrite it so that it uses the “data” (see getData()) just like the frontend code? This way this won’t break when we switch to Fiber.
There was a problem hiding this comment.
Got it. I rewrote it with getData and getDataFiber, I refer to this implementation for check Fiber.
| (prev, curr) => prev ? prev[curr] : null, | ||
| window | ||
| ); | ||
| var inst = el._reactInternalInstance; |
There was a problem hiding this comment.
Sorry I wasn't clear, I didn't mean importing getData itself. I meant accessing the node data structure (that is generated by getData when renderer attaches). You are still accessing the "opaque nodes" (el) which is a problem (they're implementation details).
Agent has access to "data" in this.elementData. However I don't think it's meant to be a public field.
I would look at showComponentSource in Panel instead. This looks like related code (not sure if it's used though). Perhaps this is a better place to implement it, and somehow inject an implementation there?
There was a problem hiding this comment.
I would look at showComponentSource in Panel instead.
I tried node.get('source') just worked in Panel, obviously I did something unnecessarily. 😂
Also, I think it would be great if we just do launchEditor in Electron part, this will don't need go to RN process (call openFileInEditor) and support non-RN platforms, what do you think?
There was a problem hiding this comment.
I tried node.get('source') just worked in Panel, obviously I did something unnecessarily. 😂
Sounds awesome!
I think it would be great if we just do launchEditor in Electron part, this will don't need go to RN process (call openFileInEditor) and support non-RN platforms, what do you think?
I don’t fully understand but if it’s better, go for it!
jhen0409
force-pushed
the
rn-show-source
branch
from
dbf10b5 to
0dd9ffc
Compare
jhen0409
changed the title
Show Source to open file in editorShow Source to open file in editor
|
@gaearon I changed this PR as #588 (comment).
Due to I removed |
jhen0409
force-pushed
the
rn-show-source
branch
from
0dd9ffc to
6f9318b
Compare
|
This looks awesome. I love it. Thanks! |
|
Please see also: #625. |
4 participants
This PR made click
Show Sourceto open file in editor in Electron part, if we usedbabel-plugin-transform-react-jsx-source):launchEditor, it's mirror from react-native packager, have changes:projectRoots, we shouldn't get it here