-
Notifications
You must be signed in to change notification settings - Fork 15
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 backend from 4.17.0 to 4.24.3 #796
Update React DevTools backend from 4.17.0 to 4.24.3 #796
Conversation
I am not yet sure how to test this 😁 Tried following the process outlined here but Edit 1: There were a few missing setup steps for the M1:
arch -arm64 brew install rustup-init
rustup-init
rustup default stable-x86_64-apple-darwin
cargo install cbindgen
arch -arm64 brew install nasm Then I was able to build with the following: MACH_USE_SYSTEM_PYTHON=/usr/bin/python \
RUSTC_BOOTSTRAP=qcms \
node build Edit 2: This command took about 35 minutes to run but eventually finished without any obvious error. No idea how to test or use what it built yet. 😁 Edit 3: I see there was a |
7d3116b
to
d0eeff9
Compare
Finally able to get the
In the Terminal I see slightly more output:
Just to rule out this change, I built from the HEAD of the |
For the backend to replay a recording it needs the browser executable with which the recording was made, so to test a locally built browser you need to use a local backend (or upload the browser to prod or cloud-dev, but I don't know if this is possible yet).
but I don't know how to proceed from here. |
@hbenl Ah, right. That may be because my exports.installHook = installHook; I fixed that just now and pushed. The process of updating Maybe we could pair and look at this together? I'm not able to build the backend currently because of Kubernetes issues so I'm temporarily blocked from verifying this locally. |
@hbenl Can you share that Replay with me? |
No, I can't because I used a local backend. |
@hbenl When you're testing the replay, are you using the changes from this PR replayio/devtools#6075? I suspect the "not a function" is the newer |
No I wasn't, I'll try that out. |
Just to clarify, did you upgrade both
Yes. That's expected because some of the APIs DevTools is using (in this case |
Yes. I tried with a newer experimental version (0.0.0-experimental-af730436c-20220405) but now I get
when running |
That's...interesting. What's your |
Gist for my |
@hbenl and I tested and verified that the changes in this PR and replayio/devtools#6075 are compatible. See that PR for remaining blockers to landing this. |
Relates to replayio/devtools#6075
This PR follows the steps outlined here to update the React DevTools (Firefox)
react_devtools_backend.js
:b9964684bd8c909fc3d88f1cd47aa1f45ea7ba32
adb8ebc927ea091ba5ffba6a9f30dbe62eaee0c5
The update instructions don't mention how the
hook.js
file was created. Here's what I did to update it:packages/react-devtools-shared/src/hook.js
in the React repo.__DEV__
,__EXTENSION__
, and__TEST__
constants at the top of the file. (Normally these are injected during build time on the React DevTools side.)import
statement at the top of the file. (The__EXTENSION__ === true
value prevents this import from being used anyway.)exports.installHook = installHook;
statement at the bottom of the file.To compare the changes afterward to the changes made to the source file between versions, I used the following command:
The result of this diff cam be viewed here.
I would say the process of updating the
hook.js
file seems pretty fragile and should be improved going forward if possible.