Skip to content

Commit 1e8aa81

Browse files
author
Brian Vaughn
committed
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.
1 parent c6fb743 commit 1e8aa81

File tree

6 files changed

+70
-45
lines changed

6 files changed

+70
-45
lines changed

packages/react-devtools-core/src/launchEditor.js renamed to packages/react-devtools-core/src/editor.js

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,31 +102,44 @@ function guessEditor(): Array<string> {
102102

103103
let childProcess = null;
104104

105-
export default function launchEditor(
105+
export function getValidFilePath(
106106
maybeRelativePath: string,
107-
lineNumber: number,
108107
absoluteProjectRoots: Array<string>
109-
) {
108+
): string | null {
110109
// We use relative paths at Facebook with deterministic builds.
111110
// This is why our internal tooling calls React DevTools with absoluteProjectRoots.
112111
// If the filename is absolute then we don't need to care about this.
113-
let filePath;
114112
if (isAbsolute(maybeRelativePath)) {
115113
if (existsSync(maybeRelativePath)) {
116-
filePath = maybeRelativePath;
114+
return maybeRelativePath;
117115
}
118116
} else {
119117
for (let i = 0; i < absoluteProjectRoots.length; i++) {
120118
const projectRoot = absoluteProjectRoots[i];
121119
const joinedPath = join(projectRoot, maybeRelativePath);
122120
if (existsSync(joinedPath)) {
123-
filePath = joinedPath;
124-
break;
121+
return joinedPath;
125122
}
126123
}
127124
}
128125

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) {
130143
return;
131144
}
132145

packages/react-devtools-core/src/standalone.js

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { Server } from 'ws';
1414
import { existsSync, readFileSync } from 'fs';
1515
import { installHook } from 'src/hook';
1616
import DevTools from 'src/devtools/views/DevTools';
17-
import launchEditor from './launchEditor';
17+
import { doesFilePathExist, launchEditor } from './editor';
1818
import { __DEBUG__ } from 'src/constants';
1919

2020
import type { FrontendBridge } from 'src/bridge';
@@ -85,16 +85,31 @@ function reload() {
8585
root.render(
8686
createElement(DevTools, {
8787
bridge: ((bridge: any): FrontendBridge),
88+
canViewElementSourceFunction,
8889
showTabBar: true,
8990
store: ((store: any): Store),
9091
warnIfLegacyBackendDetected: true,
9192
viewElementSourceFunction,
92-
viewElementSourceRequiresFileLocation: true,
9393
})
9494
);
9595
}, 100);
9696
}
9797

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+
98113
function viewElementSourceFunction(
99114
id: number,
100115
inspectedElement: InspectedElement
@@ -171,10 +186,7 @@ function initialize(socket: WebSocket) {
171186
socket.close();
172187
});
173188

174-
store = new Store(bridge, {
175-
supportsNativeInspection: false,
176-
supportsViewSource: projectRoots.length > 0,
177-
});
189+
store = new Store(bridge, { supportsNativeInspection: false });
178190

179191
log('Connected');
180192
reload();

src/devtools/store.js

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ type Config = {|
4949
supportsNativeInspection?: boolean,
5050
supportsReloadAndProfile?: boolean,
5151
supportsProfiling?: boolean,
52-
supportsViewSource?: boolean,
5352
|};
5453

5554
export type Capabilities = {|
@@ -125,7 +124,6 @@ export default class Store extends EventEmitter<{|
125124
_supportsNativeInspection: boolean = false;
126125
_supportsProfiling: boolean = false;
127126
_supportsReloadAndProfile: boolean = false;
128-
_supportsViewSource: boolean = true;
129127

130128
// Total number of visible elements (within all roots).
131129
// Used for windowing purposes.
@@ -157,15 +155,13 @@ export default class Store extends EventEmitter<{|
157155
supportsNativeInspection,
158156
supportsProfiling,
159157
supportsReloadAndProfile,
160-
supportsViewSource,
161158
} = config;
162159
if (supportsCaptureScreenshots) {
163160
this._supportsCaptureScreenshots = true;
164161
this._captureScreenshots =
165162
localStorageGetItem(LOCAL_STORAGE_CAPTURE_SCREENSHOTS_KEY) === 'true';
166163
}
167164
this._supportsNativeInspection = supportsNativeInspection !== false;
168-
this._supportsViewSource = supportsViewSource !== false;
169165
if (supportsProfiling) {
170166
this._supportsProfiling = true;
171167
}
@@ -365,10 +361,6 @@ export default class Store extends EventEmitter<{|
365361
return this._supportsReloadAndProfile && this._isBackendStorageAPISupported;
366362
}
367363

368-
get supportsViewSource(): boolean {
369-
return this._supportsViewSource;
370-
}
371-
372364
containsElement(id: number): boolean {
373365
return this._idToElement.get(id) != null;
374366
}

src/devtools/views/Components/SelectedElement.js

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,10 @@ export type Props = {||};
3434
export default function SelectedElement(_: Props) {
3535
const { inspectedElementID } = useContext(TreeStateContext);
3636
const dispatch = useContext(TreeDispatcherContext);
37-
const { isFileLocationRequired, viewElementSourceFunction } = useContext(
38-
ViewElementSourceContext
39-
);
37+
const {
38+
canViewElementSourceFunction,
39+
viewElementSourceFunction,
40+
} = useContext(ViewElementSourceContext);
4041
const bridge = useContext(BridgeContext);
4142
const store = useContext(StoreContext);
4243
const { dispatch: modalDialogDispatch } = useContext(ModalDialogContext);
@@ -90,11 +91,14 @@ export default function SelectedElement(_: Props) {
9091
}
9192
}, [inspectedElement, viewElementSourceFunction]);
9293

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).
9396
const canViewSource =
94-
inspectedElement &&
97+
inspectedElement !== null &&
9598
inspectedElement.canViewSource &&
9699
viewElementSourceFunction !== null &&
97-
(!isFileLocationRequired || inspectedElement.source !== null);
100+
(canViewElementSourceFunction === null ||
101+
canViewElementSourceFunction(inspectedElement));
98102

99103
const isSuspended =
100104
element !== null &&
@@ -201,16 +205,14 @@ export default function SelectedElement(_: Props) {
201205
>
202206
<ButtonIcon type="log-data" />
203207
</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>
214216
</div>
215217

216218
{inspectedElement === null && (

src/devtools/views/Components/ViewElementSourceContext.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@
22

33
import { createContext } from 'react';
44

5-
import type { ViewElementSource } from 'src/devtools/views/DevTools';
5+
import type {
6+
CanViewElementSource,
7+
ViewElementSource,
8+
} from 'src/devtools/views/DevTools';
69

710
export type Context = {|
8-
isFileLocationRequired: boolean,
11+
canViewElementSourceFunction: CanViewElementSource | null,
912
viewElementSourceFunction: ViewElementSource | null,
1013
|};
1114

src/devtools/views/DevTools.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,19 @@ export type ViewElementSource = (
3232
id: number,
3333
inspectedElement: InspectedElement
3434
) => void;
35+
export type CanViewElementSource = (
36+
inspectedElement: InspectedElement
37+
) => boolean;
3538

3639
export type Props = {|
3740
bridge: FrontendBridge,
3841
browserTheme?: BrowserTheme,
42+
canViewElementSourceFunction?: ?CanViewElementSource,
3943
defaultTab?: TabID,
4044
showTabBar?: boolean,
4145
store: Store,
4246
warnIfLegacyBackendDetected?: boolean,
4347
viewElementSourceFunction?: ?ViewElementSource,
44-
viewElementSourceRequiresFileLocation?: boolean,
4548

4649
// This property is used only by the web extension target.
4750
// 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];
7578
export default function DevTools({
7679
bridge,
7780
browserTheme = 'light',
81+
canViewElementSourceFunction = null,
7882
defaultTab = 'components',
7983
componentsPortalContainer,
8084
overrideTab,
@@ -83,8 +87,7 @@ export default function DevTools({
8387
showTabBar = false,
8488
store,
8589
warnIfLegacyBackendDetected = false,
86-
viewElementSourceFunction,
87-
viewElementSourceRequiresFileLocation = false,
90+
viewElementSourceFunction = null,
8891
}: Props) {
8992
const [tab, setTab] = useState(defaultTab);
9093
if (overrideTab != null && overrideTab !== tab) {
@@ -93,10 +96,10 @@ export default function DevTools({
9396

9497
const viewElementSource = useMemo(
9598
() => ({
96-
isFileLocationRequired: viewElementSourceRequiresFileLocation,
97-
viewElementSourceFunction: viewElementSourceFunction || null,
99+
canViewElementSourceFunction,
100+
viewElementSourceFunction,
98101
}),
99-
[viewElementSourceFunction, viewElementSourceRequiresFileLocation]
102+
[canViewElementSourceFunction, viewElementSourceFunction]
100103
);
101104

102105
return (

0 commit comments

Comments
 (0)