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

Update react-devtools-inline, react, and react-dom dependencies #6075

Merged
merged 7 commits into from
Apr 7, 2022
Merged

Update react-devtools-inline, react, and react-dom dependencies #6075

merged 7 commits into from
Apr 7, 2022

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Mar 30, 2022

Resolves #5344 and #4578; relates to replayio/gecko-dev#796

This react-devtools-inline update introduces a newer bridge protocol version than what is currently being embedded in the backend. That means older Replay sessions containing React data won't work with it.

I think the best short-term solution for this is for the devtools/client to check the protocol version embedded in the Replay, and load the react-devtools-inline version that supports it. This is a little hacky but it takes the pressure off of our ability to upgrade React DevTools without breaking backwards compat.

Checklist

  • Update react-devtools-inline to latest (4.24.3).
  • Update backend react-devtools-inline as well: Update React DevTools backend from 4.17.0 to 4.24.3  gecko-dev#796
  • Update frontend to check protocol and dynamically load the React DevTools package that supports that version.
  • Fix TypeScript types
  • Add inline comments explaining what/why
  • Maybe add tests (once we have shared Replay sessions with the new protocol version?)

@vercel
Copy link

vercel bot commented Mar 30, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/recordreplay/devtools/5y335EN3q5zb8rvRGJDcWgJTawgN
✅ Preview: https://devtools-git-fork-bvaughn-react-devtools-inline-recordreplay.vercel.app

@jasonLaster
Copy link
Collaborator

@bvaughn makes sense.

Here are the docs on updating the react_devtools_backend. Updating the backend is sufficient for being able to land this PR and view the latest replays.

re backward compatibility: agreed. If the frontend knows the version we should be able to load it. There are some funny hoops to jump through because the react-devtools-inline is currently in our package.json, so i'm not too sure how we'd get multiple versions, but that feels more of an opsy challenge. Related, we don't currently have backward compat on the q2 roadmap because we assumed Component Stacks and some other features took precedence, but that's all flexible.

@hbenl
Copy link
Contributor

hbenl commented Apr 1, 2022

Perhaps a solution to this could be to change our backend profile to include metadata about which backend version was used to record the "operations" message.

FYI: we're already fetching the ReactDevTools protocol version by sending the getBridgeProtocol message to the backend in the recording:
https://github.com/RecordReplay/devtools/blob/b36e7f0e2188a1a8788d92a4b8e1b01d93131b22/src/ui/components/SecondaryToolbox/ReactDevTools.tsx#L82-L83

@bvaughn
Copy link
Contributor Author

bvaughn commented Apr 6, 2022

@hbenl and I tested and verified that the changes in this PR and replayio/gecko-dev#796 are compatible. The remaining trouble is that– if we were to land these PRs, old Replays would "break" due to the React DevTools protocol version.

I'm going to see if I can't figure out a workaround to make the newer DevTools frontend support the older pre-recorded profiles.

@bvaughn
Copy link
Contributor Author

bvaughn commented Apr 6, 2022

I think the short term path forward here is to embed both versions of the react-devtools-inline package (4.17 which supports the older bridge protocol version and 4.24 which supports the newer one). Then we can look at the Replay and load the right frontend version based on the backend protocol that was captured.

This is kind of hacky but it unblocks us to update React DevTools.

I'm going to push this as-is so @hbenl can maybe verify that it works with the newer protocol version. (I can confirm that it works with an older protocol version.) I still need to clean things up (e.g. TypeScript types, inline comments, etc) but I'll do that a bit later. Need to pack and head to the airport shortly.

Copy link
Contributor

@hbenl hbenl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for me with recordings made with both the old and the new backend. The only issue is that the ReactDevTools UI is not updated when jumping to a different point in time.

@bvaughn
Copy link
Contributor Author

bvaughn commented Apr 7, 2022

Okay. I think things should work more now like they used to, except for the async module-loading hop. Sorry for not catching the jumping-between-points issue during my smoke testing. I assumed that if DevTools loaded once it would work as usual from there on out– because of my mental model of how it runs in the browser under normal conditions.

@bvaughn
Copy link
Contributor Author

bvaughn commented Apr 7, 2022

As I start to understand the way Replay and React DevTools work together here, I realize that there's a small logic bug in the mount-only effect that might "break" things if we try to get the protocol version from a point too early in the Replay session. Going to make one more small change to account for this.

@bvaughn bvaughn merged commit 6f555a6 into replayio:master Apr 7, 2022
@bvaughn bvaughn deleted the react-devtools-inline branch April 7, 2022 14:56
bvaughn added a commit to replayio/gecko-dev that referenced this pull request Apr 7, 2022
Update React DevTools backend from 4.17.0 to 4.24.3 

Relates to frontend PR replayio/devtools#6075

This commit updates the React DevTools (Firefox) `react_devtools_backend.js`:
* From: version 4.17.0 (created on 08/24/2021 from revision b9964684bd8c909fc3d88f1cd47aa1f45ea7ba32)
* To: version 4.24.3 (created on 03/29/2022 from revision adb8ebc927ea091ba5ffba6a9f30dbe62eaee0c5)

This commit also updates `hook.js` by doing the following:
* Opened `packages/react-devtools-shared/src/hook.js` in the React repo.
* Stripped the Flow annotations from the file (using the Babel REPL for convenience.)
* Manually added the `__DEV__`, `__EXTENSION__`, and `__TEST__` constants at the top of the file.
* Manually deleted the import statement at the top of the file.
* Manually re-add the `exports.installHook = installHook;` statement at the end of the file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump React DevTools
3 participants