Adding code necessary for React Native support #165

Gongreg · 2019-01-09T22:01:33Z

In React Native debugger we create our own Bridge that we want to use with apollo dev tools.
I've updated the code to allow to setup custom bridge.

The main question I have is uuid in check versions is used for? This code is not easy to implement in React Native, since it doesn't have localStorage but instead asyncStorage which is, well, async.

Of course for RN we also need to solve #160.

Displays panel if __APOLLO_DEVTOOLS_SHOULD_DISPLAY_PANEL__ is set to true. Moved out hook to the backend files, so we could reuse src/backend/index in react native too.

… debugger.

# Conflicts: # shells/dev/src/backend.js # shells/webextension/src/backend.js

apollo-cla · 2019-01-09T22:01:34Z

@Gongreg: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

benjamn · 2019-01-09T22:21:08Z

src/backend/checkVersions.js

-}
+let uuid = genUuid();

 export const checkVersions = (hook, bridge) => {


The main question I have is uuid in check versions is used for? This code is not easy to implement in React Native, since it doesn't have localStorage but instead asyncStorage which is, well, async.

It seems like checkVersions ultimately does an async fetch call, so perhaps we could store the uuid using a generic async local storage API (backed by sync localStorage in browsers, but asyncStorage in RN), and then just hide the async access within the already-async behavior of checkVersions?

Great idea @benjamn, I will implement it today!

Gongreg · 2019-01-10T09:54:43Z

Hey, @benjamn,
I've updated the code. I see that for devtools version the version is taken out of manifest.json.

I am not sure it is the right thing to do to try to get it the same way in react native.
So what I assume is that the version will be in package.json (since it has to have a version). Is that okay?

cheapsteak

This looks great 👍
Would appreciate if you could add a few comments to clarify some things that might not make sense unless you think about the package as something that'll be imported from somewhere else 🙏

cheapsteak · 2019-01-25T20:38:36Z

src/backend/index.js

 let connected;
+let storage;
+
+export const sendBridgeReady = () => {


Would you mind adding a comment here to explain why this function exists and is exported instead of inlined into the call site 🙏? I assume this is so the react native debugger can import and call this :)

cheapsteak · 2019-01-25T20:40:22Z

shells/webextension/src/devtools-background.js

  panelShown = false;
  chrome.devtools.inspectedWindow.eval(
-    `!!(window.__APOLLO_DEVTOOLS_GLOBAL_HOOK__.ApolloClient);`,
+    `!!(window.__APOLLO_DEVTOOLS_GLOBAL_HOOK__.ApolloClient || window.__APOLLO_DEVTOOLS_SHOULD_DISPLAY_PANEL__);`,


Would you mind adding a comment here as well to explain what __APOLLO_DEVTOOLS_SHOULD_DISPLAY_PANEL__ is?

Hey, since there is no Apollo client in the window context of react native debugger (the react native code with apollo is ran in a separate worker), we can't check for APOLLO_DEVTOOLS_GLOBAL_HOOK. To make it explicit I add a separate flag.

Gongreg · 2019-01-25T20:52:35Z

Hey @cheapsteak, I've added the comments :)

cheapsteak

Thanks so much for the contribution!
Looking forward to seeing this work with React Native Debugger :D
Do we need to publish a new version to npm after merging?

Gongreg · 2019-01-25T20:59:11Z

Yea, that would be great.
Then I will do final checks with the react native debugger itself and it should be good to go :)

(I've worked on two projects, while doing changes both in checked out projects and in node_modules, in two different machines, so there is a big chance I might have missed something :D)

Can you notify me when the new npm version is published?

cheapsteak · 2019-01-25T21:03:34Z

That sounds great :D
Yup I can do that

cheapsteak · 2019-01-25T21:13:54Z

@Gongreg published under 2.1.7-alpha.0 for now for testing 🙏

Gongreg · 2019-01-25T21:52:35Z

hey @cheapsteak, seems like the release didn't go well. I still get the code without the PR in 2.1.7-alpha.0 .

https://registry.npmjs.org/apollo-client-devtools/-/apollo-client-devtools-2.1.7-alpha.0.tgz

cheapsteak · 2019-01-25T22:04:00Z

@Gongreg apologies, my bad 🤦‍♂️
just published alpha.1

Gongreg · 2019-01-25T22:19:55Z

@cheapsteak Just tested things out. It works! :) Going to try to get the react-native-debugger pr to be merged as soon as possible :)

Gongreg · 2019-01-25T22:30:05Z

@cheapsteak , one more issue. We need to have dist directory in shells/webextension, since we use the panel in react native debugger.

cheapsteak · 2019-01-26T02:21:57Z

Ah, that makes sense
We could probably add that as a prepublish hook, and also add it to gitignore
You might have more familiarity with this repo than Justin and I do at the moment 😛, do you know if running the build script would be sufficient? Or would we need to add a new script?

Gongreg · 2019-01-28T09:24:24Z

Truth to be told I don't know which option is better. I've never worked on the module before this PR. But adding it as a build step sounds reasonable.

I will create a separate issue and assign you, so it wouldn't be forgotten.

justinanastos · 2019-01-28T14:22:14Z

#169

cheapsteak · 2019-01-28T16:11:35Z

Published alpha.2 that includes the dist folder
https://unpkg.com/[email protected]/shells/webextension/dist/

Gongreg added 4 commits December 12, 2018 10:21

Adding support for react-native

4ac6562

Adding support for react-native.

ab9dc4e

Displays panel if __APOLLO_DEVTOOLS_SHOULD_DISPLAY_PANEL__ is set to true. Moved out hook to the backend files, so we could reuse src/backend/index in react native too.

Allows to set up custom hook, so we can set our own from react native…

5494ff9

… debugger.

Merge remote-tracking branch 'origin/apollo' into apollo

7a288b3

# Conflicts: # shells/dev/src/backend.js # shells/webextension/src/backend.js

Gongreg requested review from hwillson and jbaxleyiii as code owners January 9, 2019 22:01

benjamn reviewed Jan 9, 2019

View reviewed changes

Supports async storage in checkVersions.js

8e79b30

This was referenced Jan 10, 2019

Adding apollo dev tools. jhen0409/react-native-debugger#298

Merged

[Feature request] Add Apollo Dev Tools jhen0409/react-native-debugger#177

Closed

Adding apollo. Gongreg/react-native-debugger#1

Closed

hwillson mentioned this pull request Jan 17, 2019

Publishing as npm module #160

Closed

Gongreg mentioned this pull request Jan 17, 2019

React Native implementation #6

Closed

hwillson requested review from cheapsteak and justinanastos and removed request for hwillson and jbaxleyiii January 25, 2019 10:22

cheapsteak reviewed Jan 25, 2019

View reviewed changes

Gongreg added 2 commits January 25, 2019 22:49

Adding comment to explain why we expose sendBridgeReady method

24a63f2

Explained why I've added a separate flag for displaying the panel.

bb0c4e8

cheapsteak approved these changes Jan 25, 2019

View reviewed changes

cheapsteak merged commit cc05e07 into apollographql:master Jan 25, 2019

Gongreg deleted the apollo branch January 25, 2019 22:19

sgrund14 mentioned this pull request Feb 28, 2023

react-native-debugger compatibility warrenday/graphql-network-inspector#84

Closed

Adding code necessary for React Native support #165

Adding code necessary for React Native support #165

Uh oh!

Conversation

Gongreg commented Jan 9, 2019

Uh oh!

apollo-cla commented Jan 9, 2019

Uh oh!

benjamn Jan 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gongreg Jan 10, 2019

Choose a reason for hiding this comment

Uh oh!

Gongreg commented Jan 10, 2019

Uh oh!

cheapsteak left a comment

Choose a reason for hiding this comment

Uh oh!

cheapsteak Jan 25, 2019

Choose a reason for hiding this comment

Uh oh!

cheapsteak Jan 25, 2019

Choose a reason for hiding this comment

Uh oh!

Gongreg Jan 25, 2019

Choose a reason for hiding this comment

Uh oh!

Gongreg commented Jan 25, 2019

Uh oh!

cheapsteak left a comment

Choose a reason for hiding this comment

Uh oh!

Gongreg commented Jan 25, 2019

Uh oh!

cheapsteak commented Jan 25, 2019

Uh oh!

cheapsteak commented Jan 25, 2019

Uh oh!

Gongreg commented Jan 25, 2019

Uh oh!

cheapsteak commented Jan 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gongreg commented Jan 25, 2019

Uh oh!

Gongreg commented Jan 25, 2019

Uh oh!

cheapsteak commented Jan 26, 2019

Uh oh!

Gongreg commented Jan 28, 2019

Uh oh!

justinanastos commented Jan 28, 2019

Uh oh!

cheapsteak commented Jan 28, 2019

Uh oh!

Uh oh!

benjamn Jan 9, 2019 •

edited

Loading

cheapsteak commented Jan 25, 2019 •

edited

Loading