-
Notifications
You must be signed in to change notification settings - Fork 47.9k
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
WIP: Show unsupported screen in dev tools for React version < 15.0.0 #16720
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the place we most likely want to do this check is here:
react/packages/react-devtools-shared/src/backend/index.js
Lines 50 to 56 in 2ce5801
// Inject any not-yet-injected renderers (if we didn't reload-and-profile) | |
if (!rendererInterface) { | |
if (typeof renderer.findFiberByHostInstance === 'function') { | |
rendererInterface = attach(hook, id, renderer, global); | |
} else { | |
rendererInterface = attachLegacy(hook, id, renderer, global); | |
} |
In the else branch (where we call attachLegacy
) we should try to detect v14 and older (maybe using the ComponentTree
check I mentioned). If we detect an unsupported version, we should:
- send a message over the bridge so the frontend knows to show a modal
- Not call any of the "attach" methods since they would just runtime error anyway.
store = new Store(bridge, {supportsNativeInspection: false}); | ||
store = new Store(bridge, { | ||
supportsNativeInspection: false, | ||
supportsReact: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this param name is a bit confusing. React DevTools always "support React" 😄 We just want to detect an unsupported version.
I think this probably doesn't belong as a store capability.
@@ -130,6 +131,7 @@ function createPanelIfReactLoaded() { | |||
isProfiling, | |||
supportsReloadAndProfile: isChrome, | |||
supportsProfiling, | |||
supportsReact: renderers.get(1) && !!renderers.get(1).ComponentTree, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above comment. I don't think this belongs as a store capability, so this can be reverted 😄
<SettingsModal /> | ||
</div> | ||
) : ( | ||
<UnsupportedReactVersion /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use the ModalDialogContext
to show a message if we detect an unsupported React version. We do a similar thing for React Native in WarnIfLegacyBackendDetected
actually. Conceptually the two are kind of the same.
Thanks! |
Closing in favor of #16897 |
refs #16462
Before submitting a pull request, please make sure the following is done:
master
.yarn
in the repository root.yarn test
). Tip:yarn test --watch TestName
is helpful in development.yarn test-prod
to test in the production environment. It supports the same options asyarn test
.yarn debug-test --watch TestName
, openchrome://inspect
, and press "Inspect".yarn prettier
).yarn lint
). Tip:yarn linc
to only check changed files.yarn flow
).Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html