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

Replace Fusebox setClientMetadata handshake with ReactNativeApplicationModel #139

Merged

Conversation

huntie
Copy link

@huntie huntie commented Nov 26, 2024

Summary

Depends on facebook/react-native#47962.

In an aim to simplify the rn_fusebox.ts entry point, drops our use of FuseboxClient.setClientMetadata as a RNDT-client-identifying handshake, in favour ReactNativeApplication.enable.

  • Replace FuseboxClientMetadataModel with existing ReactNativeApplicationModel
    • Now auto-inits, registration call is moved to rn_fusebox.ts (IMO, correctness fix that now includes this model only for this entrypoint).
    • Migrate rnPerfMetrics calls over (unchanged).
  • Remove CDP definitions for FuseboxClient domain.

Test plan

[Meta internal] See D66501027.

  • This change maintains backwards compatibility with previous Local Storage data (if modifying settings, experiments, or other persisted client state).

Upstreaming plan

  • This commit should be sent as a patch to the upstream devtools-frontend repo. I've reviewed the contribution guide.
  • This commit is React Native-specific and cannot be upstreamed.

@huntie huntie requested review from robhogan and vzaidman November 26, 2024 18:04
huntie added a commit to huntie/react-native that referenced this pull request Nov 28, 2024
Summary:
Follows facebook#47962 and depends on facebookexperimental/rn-chrome-devtools-frontend#139.

Updates the modern debugger server to no longer respond to `FuseboxClient` messages — namely `FuseboxClient.setClientMetadata`. This method was previously used to identify the React Native DevTools frontend, replaced/inferred by `ReactNativeApplication.enable` instead.

Changelog:
[General][Breaking] - The `FuseboxClient.setClientMetadata` CDP method is removed. Instead, use `ReactNativeApplication.enable`.

Differential Revision: D66575324
Comment on lines +35 to +46
void this.#agent.invoke_enable()
.then(result => {
const maybeError = result.getError();
const success = !maybeError;
Host.rnPerfMetrics.fuseboxSetClientMetadataFinished(success, maybeError);
})
.catch(reason => {
const success = false;
Host.rnPerfMetrics.fuseboxSetClientMetadataFinished(success, reason);
});
Copy link

Choose a reason for hiding this comment

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

I believe you are missing Host.rnPerfMetrics.fuseboxSetClientMetadataStarted(); somewhere here

Copy link
Author

Choose a reason for hiding this comment

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

Yes! Updated.

@huntie huntie force-pushed the drop-fusebox-client-domain branch from eab7902 to e7818f6 Compare December 2, 2024 12:02
@huntie huntie merged commit 8831e78 into facebookexperimental:main Dec 2, 2024
2 checks passed
@huntie huntie deleted the drop-fusebox-client-domain branch December 2, 2024 12:04
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Dec 2, 2024
Summary:
Pull Request resolved: #48004

Follows #47962 and depends on facebookexperimental/rn-chrome-devtools-frontend#139.

Updates the modern debugger server to no longer respond to `FuseboxClient` messages — namely `FuseboxClient.setClientMetadata`. This method is replaced by `ReactNativeApplication.enable` for identifying the React Native DevTools frontend.

Changelog:
[General][Breaking] - The `FuseboxClient.setClientMetadata` CDP method is removed. Instead, use `ReactNativeApplication.enable`.

Reviewed By: rubennorte

Differential Revision: D66575324

fbshipit-source-id: f2b4cbacd857931832d89305510f5aaf51df483a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants