-
Notifications
You must be signed in to change notification settings - Fork 50.3k
WIP: Show unsupported screen in dev tools for React version < 15.0.0 #16720
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
Conversation
bvaughn
left a comment
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.
| 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.yarnin the repository root.yarn test). Tip:yarn test --watch TestNameis helpful in development.yarn test-prodto 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 lincto only check changed files.yarn flow).Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html