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

[DevTools] Extension reports logged events when feature flag is enabled #22475

Merged
merged 3 commits into from Sep 30, 2021

Conversation

jstejada
Copy link
Contributor

@jstejada jstejada commented Sep 30, 2021

Summary

This commit implements an event logger for the DevTools extensions (react-devtools-extension), which will report events that are logged from the DevTools runtime to an iframe that knows how to process those events. Note that this will only happen when the enableLogger feature flag is enabled, which will only be enabled for internal (fb-only) builds of DevTools.

Note that for now, the enableLogger feature flag is fully disabled, so this commit should have no observable effect on DevTools after landing.

How did you test this change?

  • yarn flow
  • yarn test
  • yarn test-build-devtools
  • manually enabled enableLogger feature flag and verified that events were correctly communicated to iframe that loads an internal endpoint.

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

This looks like it should work 👍🏼

I wish we could add some basic e2e tests for this sort of thing. (No idea how to do that for an extension, unfortunately.)

@@ -73,6 +75,7 @@ module.exports = {
'process.env.DEVTOOLS_PACKAGE': `"react-devtools-extensions"`,
'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
'process.env.GITHUB_URL': `"${GITHUB_URL}"`,
'process.env.LOGGING_URL': `"${LOGGING_URL}"`,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I don't think we need this in the backend config, do we? This variable is only referenced in code imported form main.js which is built by the frontend config.

@jstejada
Copy link
Contributor Author

I wish we could add some basic e2e tests for this sort of thing. (No idea how to do that for an extension, unfortunately.)

yeah, it would be nice to set something up here. for the time being i manually verified that events were going all the way through

@jstejada jstejada merged commit 75b9869 into facebook:main Sep 30, 2021
@jstejada jstejada deleted the connect-logging branch September 30, 2021 17:48
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
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.

None yet

3 participants