[DevTools] [Context] Legacy Context #16617
Conversation
hristo-kanchev
changed the title
|
Is it possible to add some sort of a test? |
Tests ✅ |
|
@gaearon Did you manage to check the tests? |
| canViewSource = true; | ||
| if (stateNode && stateNode.context != null) { | ||
| context = stateNode.context; | ||
| const shouldHideContext = |
There was a problem hiding this comment.
This could probably use an explanatory inline comment. (I'll add one.)
| import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; | ||
| import type Store from 'react-devtools-shared/src/devtools/store'; | ||
| import React, {Component, createContext} from 'react'; | ||
| import {Fragment} from '../../../../../../../../../../Applications/WebStorm.app/Contents/plugins/JavaScriptLanguage/jsLanguageServicesImpl/flow/react'; |
There was a problem hiding this comment.
Freakin' WebStorm 😆 #NeverTrustYourEditor
What's this comment about? Is this PR safe to merge or do we need to check on something first? |
|
I think this was meant as “@gaearon can you review again now that tests were added”. Lgtm |
| canViewSource: type === ElementTypeClass || type === ElementTypeFunction, | ||
|
|
||
| // Only legacy context exists in legacy versions. | ||
| hasLegacyContext: true, |
There was a problem hiding this comment.
Should we also hide empty one though in the legacy mode?
There was a problem hiding this comment.
This value isn't used to hide the context label in the front end.. It just specifies whether the context is modern vs legacy. I guess it's a little weird to have a boolean value for this. Maybe we should use an enum? context type: none, modern, legacy.
Anyway, an empty legacy context won't be shown on the frontend because the InspectedElementTree component hides itself if the value it's passed is null or an empty object.
Fixes: #16466 and #16679
Description:
We are now changing the context label in DevTools based on if the component is using the legacy way of adding context.