Re-enable "view source" button for standalone shell
But only do this if we can verify the element file path. This hopefully avoids the case where clicking the button does nothing because of an invalid/incomplete path.
Brian Vaughn committed
Jul 25, 2019 at 09:28 UTC
1e8aa8105ab10ee7c4ec49a405fcc7976820da4c
6 files changed
+70
-45
packages/react-devtools-core/src/editor.js
renamed
+21
-8
@@ -102,31 +102,44 @@ function guessEditor(): Array<string> {
102
103
let childProcess = null;
104
105
-export default function launchEditor(
105
+export function getValidFilePath(
106
maybeRelativePath: string,
107
- lineNumber: number,
107
absoluteProjectRoots: Array<string>
109
-) {
108
+): string | null {
109
// We use relative paths at Facebook with deterministic builds.
110
// This is why our internal tooling calls React DevTools with absoluteProjectRoots.
111
// If the filename is absolute then we don't need to care about this.
113
- let filePath;
112
if (isAbsolute(maybeRelativePath)) {
113
if (existsSync(maybeRelativePath)) {
116
- filePath = maybeRelativePath;
114
+ return maybeRelativePath;
115
}
116
} else {
117
for (let i = 0; i < absoluteProjectRoots.length; i++) {
118
const projectRoot = absoluteProjectRoots[i];
119
const joinedPath = join(projectRoot, maybeRelativePath);
120
if (existsSync(joinedPath)) {
123
- filePath = joinedPath;
124
- break;
121
+ return joinedPath;
122
}
123
}
124
}
125
129
- if (!filePath) {
126
+ return null;
127
+}
128
+
129
+export function doesFilePathExist(
130
+ maybeRelativePath: string,
131
+ absoluteProjectRoots: Array<string>
132
+): boolean {
133
+ return getValidFilePath(maybeRelativePath, absoluteProjectRoots) !== null;
134
+}
135
+
136
+export function launchEditor(
137
+ maybeRelativePath: string,
138
+ lineNumber: number,
139
+ absoluteProjectRoots: Array<string>
140
+) {
141
+ const filePath = getValidFilePath(maybeRelativePath, absoluteProjectRoots);
142
+ if (filePath === null) {
143
return;
144
}
145
packages/react-devtools-core/src/standalone.js
+18
-6
@@ -14,7 +14,7 @@ import { Server } from 'ws';
14
import { existsSync, readFileSync } from 'fs';
15
import { installHook } from 'src/hook';
16
import DevTools from 'src/devtools/views/DevTools';
17
-import launchEditor from './launchEditor';
17
+import { doesFilePathExist, launchEditor } from './editor';
18
import { __DEBUG__ } from 'src/constants';
19
20
import type { FrontendBridge } from 'src/bridge';
@@ -85,16 +85,31 @@ function reload() {
85
root.render(
86
createElement(DevTools, {
87
bridge: ((bridge: any): FrontendBridge),
88
+ canViewElementSourceFunction,
89
showTabBar: true,
90
store: ((store: any): Store),
91
warnIfLegacyBackendDetected: true,
92
viewElementSourceFunction,
92
- viewElementSourceRequiresFileLocation: true,
93
})
94
);
95
}, 100);
96
}
97
98
+function canViewElementSourceFunction(
99
+ inspectedElement: InspectedElement
100
+): boolean {
101
+ if (
102
+ inspectedElement.canViewSource === false ||
103
+ inspectedElement.source === null
104
+ ) {
105
+ return false;
106
+ }
107
+
108
+ const { source } = inspectedElement;
109
+
110
+ return doesFilePathExist(source.fileName, projectRoots);
111
+}
112
+
113
function viewElementSourceFunction(
114
id: number,
115
inspectedElement: InspectedElement
@@ -171,10 +186,7 @@ function initialize(socket: WebSocket) {
186
socket.close();
187
});
188
174
- store = new Store(bridge, {
175
- supportsNativeInspection: false,
176
- supportsViewSource: projectRoots.length > 0,
177
- });
189
+ store = new Store(bridge, { supportsNativeInspection: false });
190
191
log('Connected');
192
reload();
src/devtools/store.js
-8
@@ -49,7 +49,6 @@ type Config = {|
49
supportsNativeInspection?: boolean,
50
supportsReloadAndProfile?: boolean,
51
supportsProfiling?: boolean,
52
- supportsViewSource?: boolean,
52
|};
53
54
export type Capabilities = {|
@@ -125,7 +124,6 @@ export default class Store extends EventEmitter<{|
124
_supportsNativeInspection: boolean = false;
125
_supportsProfiling: boolean = false;
126
_supportsReloadAndProfile: boolean = false;
128
- _supportsViewSource: boolean = true;
127
128
// Total number of visible elements (within all roots).
129
// Used for windowing purposes.
@@ -157,7 +155,6 @@ export default class Store extends EventEmitter<{|
155
supportsNativeInspection,
156
supportsProfiling,
157
supportsReloadAndProfile,
160
- supportsViewSource,
158
} = config;
159
if (supportsCaptureScreenshots) {
160
this._supportsCaptureScreenshots = true;
@@ -165,7 +162,6 @@ export default class Store extends EventEmitter<{|
162
localStorageGetItem(LOCAL_STORAGE_CAPTURE_SCREENSHOTS_KEY) === 'true';
163
}
164
this._supportsNativeInspection = supportsNativeInspection !== false;
168
- this._supportsViewSource = supportsViewSource !== false;
165
if (supportsProfiling) {
166
this._supportsProfiling = true;
167
}
@@ -365,10 +361,6 @@ export default class Store extends EventEmitter<{|
361
return this._supportsReloadAndProfile && this._isBackendStorageAPISupported;
362
}
363
368
- get supportsViewSource(): boolean {
369
- return this._supportsViewSource;
370
- }
371
-
364
containsElement(id: number): boolean {
365
return this._idToElement.get(id) != null;
366
}
src/devtools/views/Components/SelectedElement.js
+17
-15
@@ -34,9 +34,10 @@ export type Props = {||};
34
export default function SelectedElement(_: Props) {
35
const { inspectedElementID } = useContext(TreeStateContext);
36
const dispatch = useContext(TreeDispatcherContext);
37
- const { isFileLocationRequired, viewElementSourceFunction } = useContext(
38
- ViewElementSourceContext
39
- );
37
+ const {
38
+ canViewElementSourceFunction,
39
+ viewElementSourceFunction,
40
+ } = useContext(ViewElementSourceContext);
41
const bridge = useContext(BridgeContext);
42
const store = useContext(StoreContext);
43
const { dispatch: modalDialogDispatch } = useContext(ModalDialogContext);
@@ -90,11 +91,14 @@ export default function SelectedElement(_: Props) {
91
}
92
}, [inspectedElement, viewElementSourceFunction]);
93
94
+ // In some cases (e.g. FB internal usage) the standalone shell might not be able to view the source.
95
+ // To detect this case, we defer to an injected helper function (if present).
96
const canViewSource =
94
- inspectedElement &&
97
+ inspectedElement !== null &&
98
inspectedElement.canViewSource &&
99
viewElementSourceFunction !== null &&
97
- (!isFileLocationRequired || inspectedElement.source !== null);
100
+ (canViewElementSourceFunction === null ||
101
+ canViewElementSourceFunction(inspectedElement));
102
103
const isSuspended =
104
element !== null &&
@@ -201,16 +205,14 @@ export default function SelectedElement(_: Props) {
205
>
206
<ButtonIcon type="log-data" />
207
</Button>
204
- {store.supportsViewSource && (
205
- <Button
206
- className={styles.IconButton}
207
- disabled={!canViewSource}
208
- onClick={viewSource}
209
- title="View source for this element"
210
- >
211
- <ButtonIcon type="view-source" />
212
- </Button>
213
- )}
208
+ <Button
209
+ className={styles.IconButton}
210
+ disabled={!canViewSource}
211
+ onClick={viewSource}
212
+ title="View source for this element"
213
+ >
214
+ <ButtonIcon type="view-source" />
215
+ </Button>
216
</div>
217
218
{inspectedElement === null && (
src/devtools/views/Components/ViewElementSourceContext.js
+5
-2
@@ -2,10 +2,13 @@
2
3
import { createContext } from 'react';
4
5
-import type { ViewElementSource } from 'src/devtools/views/DevTools';
5
+import type {
6
+ CanViewElementSource,
7
+ ViewElementSource,
8
+} from 'src/devtools/views/DevTools';
9
10
export type Context = {|
8
- isFileLocationRequired: boolean,
11
+ canViewElementSourceFunction: CanViewElementSource | null,
12
viewElementSourceFunction: ViewElementSource | null,
13
|};
14
src/devtools/views/DevTools.js
+9
-6
@@ -32,16 +32,19 @@ export type ViewElementSource = (
32
id: number,
33
inspectedElement: InspectedElement
34
) => void;
35
+export type CanViewElementSource = (
36
+ inspectedElement: InspectedElement
37
+) => boolean;
38
39
export type Props = {|
40
bridge: FrontendBridge,
41
browserTheme?: BrowserTheme,
42
+ canViewElementSourceFunction?: ?CanViewElementSource,
43
defaultTab?: TabID,
44
showTabBar?: boolean,
45
store: Store,
46
warnIfLegacyBackendDetected?: boolean,
47
viewElementSourceFunction?: ?ViewElementSource,
44
- viewElementSourceRequiresFileLocation?: boolean,
48
49
// This property is used only by the web extension target.
50
// The built-in tab UI is hidden in that case, in favor of the browser's own panel tabs.
@@ -75,6 +78,7 @@ const tabs = [componentsTab, profilerTab];
78
export default function DevTools({
79
bridge,
80
browserTheme = 'light',
81
+ canViewElementSourceFunction = null,
82
defaultTab = 'components',
83
componentsPortalContainer,
84
overrideTab,
@@ -83,8 +87,7 @@ export default function DevTools({
87
showTabBar = false,
88
store,
89
warnIfLegacyBackendDetected = false,
86
- viewElementSourceFunction,
87
- viewElementSourceRequiresFileLocation = false,
90
+ viewElementSourceFunction = null,
91
}: Props) {
92
const [tab, setTab] = useState(defaultTab);
93
if (overrideTab != null && overrideTab !== tab) {
@@ -93,10 +96,10 @@ export default function DevTools({
96
97
const viewElementSource = useMemo(
98
() => ({
96
- isFileLocationRequired: viewElementSourceRequiresFileLocation,
97
- viewElementSourceFunction: viewElementSourceFunction || null,
99
+ canViewElementSourceFunction,
100
+ viewElementSourceFunction,
101
}),
99
- [viewElementSourceFunction, viewElementSourceRequiresFileLocation]
102
+ [canViewElementSourceFunction, viewElementSourceFunction]
103
);
104
105
return (