Skip to content

Commit 51ce8bc

Browse files
author
Brian Vaughn
committed
Backed out DevTools feature flag fork in favor of global env vars
1 parent e73fb41 commit 51ce8bc

File tree

10 files changed

+47
-109
lines changed

10 files changed

+47
-109
lines changed

‎packages/react-devtools-core/webpack.backend.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,13 @@ module.exports = {
3838
'react-debug-tools': resolve(builtModulesDir, 'react-debug-tools'),
3939
'react-is': resolve(builtModulesDir, 'react-is'),
4040
scheduler: resolve(builtModulesDir, 'scheduler'),
41-
'shared/ReactFeatureFlags': 'shared/forks/ReactFeatureFlags.devtools',
4241
},
4342
},
4443
plugins: [
4544
new DefinePlugin({
4645
__DEV__: true,
46+
__PROFILE__: false,
47+
__EXPERIMENTAL__: true,
4748
'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
4849
'process.env.GITHUB_URL': `"${GITHUB_URL}"`,
4950
}),

‎packages/react-devtools-core/webpack.standalone.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ module.exports = {
3737
'react-debug-tools': resolve(builtModulesDir, 'react-debug-tools'),
3838
'react-is': resolve(builtModulesDir, 'react-is'),
3939
scheduler: resolve(builtModulesDir, 'scheduler'),
40-
'shared/ReactFeatureFlags': 'shared/forks/ReactFeatureFlags.devtools',
4140
},
4241
},
4342
node: {
@@ -49,6 +48,8 @@ module.exports = {
4948
plugins: [
5049
new DefinePlugin({
5150
__DEV__: false,
51+
__PROFILE__: false,
52+
__EXPERIMENTAL__: true,
5253
'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
5354
'process.env.GITHUB_URL': `"${GITHUB_URL}"`,
5455
'process.env.NODE_ENV': `"${NODE_ENV}"`,

‎packages/react-devtools-extensions/webpack.backend.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,13 @@ module.exports = {
3333
'react-dom': resolve(builtModulesDir, 'react-dom'),
3434
'react-is': resolve(builtModulesDir, 'react-is'),
3535
scheduler: resolve(builtModulesDir, 'scheduler'),
36-
'shared/ReactFeatureFlags': 'shared/forks/ReactFeatureFlags.devtools',
3736
},
3837
},
3938
plugins: [
4039
new DefinePlugin({
4140
__DEV__: true,
41+
__PROFILE__: false,
42+
__EXPERIMENTAL__: true,
4243
'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
4344
'process.env.GITHUB_URL': `"${GITHUB_URL}"`,
4445
}),

‎packages/react-devtools-extensions/webpack.config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,13 @@ module.exports = {
3838
'react-dom': resolve(builtModulesDir, 'react-dom'),
3939
'react-is': resolve(builtModulesDir, 'react-is'),
4040
scheduler: resolve(builtModulesDir, 'scheduler'),
41-
'shared/ReactFeatureFlags': 'shared/forks/ReactFeatureFlags.devtools',
4241
},
4342
},
4443
plugins: [
4544
new DefinePlugin({
4645
__DEV__: false,
46+
__PROFILE__: false,
47+
__EXPERIMENTAL__: true,
4748
'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
4849
'process.env.GITHUB_URL': `"${GITHUB_URL}"`,
4950
'process.env.NODE_ENV': `"${NODE_ENV}"`,

‎packages/react-devtools-inline/webpack.config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,12 @@ module.exports = {
3535
'react-dom': 'react-dom',
3636
'react-is': 'react-is',
3737
scheduler: 'scheduler',
38-
'shared/ReactFeatureFlags': 'shared/forks/ReactFeatureFlags.devtools',
3938
},
4039
plugins: [
4140
new DefinePlugin({
4241
__DEV__,
42+
__PROFILE__: false,
43+
__EXPERIMENTAL__: true,
4344
'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
4445
'process.env.GITHUB_URL': `"${GITHUB_URL}"`,
4546
'process.env.NODE_ENV': `"${NODE_ENV}"`,

‎packages/react-devtools-shared/src/__tests__/console-test.js

Lines changed: 27 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ describe('console', () => {
114114
});
115115

116116
it('should not append multiple stacks', () => {
117-
const Child = () => {
117+
const Child = ({children}) => {
118118
fakeConsole.warn('warn\n in Child (at fake.js:123)');
119119
fakeConsole.error('error', '\n in Child (at fake.js:123)');
120120
return null;
@@ -135,12 +135,12 @@ describe('console', () => {
135135

136136
it('should append component stacks to errors and warnings logged during render', () => {
137137
const Intermediate = ({children}) => children;
138-
const Parent = () => (
138+
const Parent = ({children}) => (
139139
<Intermediate>
140140
<Child />
141141
</Intermediate>
142142
);
143-
const Child = () => {
143+
const Child = ({children}) => {
144144
fakeConsole.error('error');
145145
fakeConsole.log('log');
146146
fakeConsole.warn('warn');
@@ -149,42 +149,31 @@ describe('console', () => {
149149

150150
act(() => ReactDOM.render(<Parent />, document.createElement('div')));
151151

152-
// TRICKY DevTools console override re-renders the component to intentionally trigger an error,
153-
// in which case all of the mock functions will be called an additional time.
154-
155-
expect(mockError).toHaveBeenCalledTimes(2);
156-
expect(mockError.mock.calls[0]).toHaveLength(1);
157-
expect(mockError.mock.calls[0][0]).toBe('error');
158-
expect(mockError.mock.calls[1]).toHaveLength(2);
159-
expect(mockError.mock.calls[1][0]).toBe('error');
160-
expect(normalizeCodeLocInfo(mockError.mock.calls[1][1])).toBe(
161-
'\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
162-
);
163-
164-
expect(mockLog).toHaveBeenCalledTimes(2);
152+
expect(mockLog).toHaveBeenCalledTimes(1);
165153
expect(mockLog.mock.calls[0]).toHaveLength(1);
166154
expect(mockLog.mock.calls[0][0]).toBe('log');
167-
expect(mockLog.mock.calls[1]).toHaveLength(1);
168-
expect(mockLog.mock.calls[1][0]).toBe('log');
169-
170-
expect(mockWarn).toHaveBeenCalledTimes(2);
171-
expect(mockWarn.mock.calls[0]).toHaveLength(1);
155+
expect(mockWarn).toHaveBeenCalledTimes(1);
156+
expect(mockWarn.mock.calls[0]).toHaveLength(2);
172157
expect(mockWarn.mock.calls[0][0]).toBe('warn');
173-
expect(mockWarn.mock.calls[1]).toHaveLength(2);
174-
expect(mockWarn.mock.calls[1][0]).toBe('warn');
175-
expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][1])).toEqual(
158+
expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual(
159+
'\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
160+
);
161+
expect(mockError).toHaveBeenCalledTimes(1);
162+
expect(mockError.mock.calls[0]).toHaveLength(2);
163+
expect(mockError.mock.calls[0][0]).toBe('error');
164+
expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe(
176165
'\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
177166
);
178167
});
179168

180169
it('should append component stacks to errors and warnings logged from effects', () => {
181170
const Intermediate = ({children}) => children;
182-
const Parent = () => (
171+
const Parent = ({children}) => (
183172
<Intermediate>
184173
<Child />
185174
</Intermediate>
186175
);
187-
const Child = () => {
176+
const Child = ({children}) => {
188177
React.useLayoutEffect(() => {
189178
fakeConsole.error('active error');
190179
fakeConsole.log('active log');
@@ -231,7 +220,7 @@ describe('console', () => {
231220

232221
it('should append component stacks to errors and warnings logged from commit hooks', () => {
233222
const Intermediate = ({children}) => children;
234-
const Parent = () => (
223+
const Parent = ({children}) => (
235224
<Intermediate>
236225
<Child />
237226
</Intermediate>
@@ -287,7 +276,7 @@ describe('console', () => {
287276

288277
it('should append component stacks to errors and warnings logged from gDSFP', () => {
289278
const Intermediate = ({children}) => children;
290-
const Parent = () => (
279+
const Parent = ({children}) => (
291280
<Intermediate>
292281
<Child />
293282
</Intermediate>
@@ -325,7 +314,7 @@ describe('console', () => {
325314
});
326315

327316
it('should append stacks after being uninstalled and reinstalled', () => {
328-
const Child = () => {
317+
const Child = ({children}) => {
329318
fakeConsole.warn('warn');
330319
fakeConsole.error('error');
331320
return null;
@@ -344,20 +333,17 @@ describe('console', () => {
344333
patchConsole();
345334
act(() => ReactDOM.render(<Child />, document.createElement('div')));
346335

347-
// TRICKY DevTools console override re-renders the component to intentionally trigger an error,
348-
// in which case all of the mock functions will be called an additional time.
349-
350-
expect(mockWarn).toHaveBeenCalledTimes(3);
351-
expect(mockWarn.mock.calls[2]).toHaveLength(2);
352-
expect(mockWarn.mock.calls[2][0]).toBe('warn');
353-
expect(normalizeCodeLocInfo(mockWarn.mock.calls[2][1])).toEqual(
336+
expect(mockWarn).toHaveBeenCalledTimes(2);
337+
expect(mockWarn.mock.calls[1]).toHaveLength(2);
338+
expect(mockWarn.mock.calls[1][0]).toBe('warn');
339+
expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][1])).toEqual(
354340
'\n in Child (at **)',
355341
);
356-
expect(mockError).toHaveBeenCalledTimes(3);
357-
expect(mockError.mock.calls[2]).toHaveLength(2);
358-
expect(mockError.mock.calls[2][0]).toBe('error');
359-
expect(normalizeCodeLocInfo(mockError.mock.calls[2][1])).toBe(
342+
expect(mockError).toHaveBeenCalledTimes(2);
343+
expect(mockError.mock.calls[1]).toHaveLength(2);
344+
expect(mockError.mock.calls[1][0]).toBe('error');
345+
expect(normalizeCodeLocInfo(mockError.mock.calls[1][1])).toBe(
360346
'\n in Child (at **)',
361347
);
362348
});
363-
});
349+
});

‎packages/react-devtools-shared/src/backend/console.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,10 @@ export function patch(): void {
107107
for (const {getCurrentFiber} of injectedRenderers.values()) {
108108
const current: ?Fiber = getCurrentFiber();
109109
if (current != null) {
110-
args.push(getStackByFiberInDevAndProd(current));
110+
const componentStack = getStackByFiberInDevAndProd(current);
111+
if (componentStack !== '') {
112+
args.push(componentStack);
113+
}
111114
break;
112115
}
113116
}

‎packages/react-devtools-shell/webpack.config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,13 @@ const config = {
3737
'react-debug-tools': resolve(builtModulesDir, 'react-debug-tools'),
3838
'react-is': resolve(builtModulesDir, 'react-is'),
3939
scheduler: resolve(builtModulesDir, 'scheduler'),
40-
'shared/ReactFeatureFlags': 'shared/forks/ReactFeatureFlags.devtools',
4140
},
4241
},
4342
plugins: [
4443
new DefinePlugin({
4544
__DEV__,
45+
__PROFILE__: false,
46+
__EXPERIMENTAL__: true,
4647
'process.env.GITHUB_URL': `"${GITHUB_URL}"`,
4748
'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
4849
}),

‎packages/shared/forks/ReactFeatureFlags.devtools.js

Lines changed: 0 additions & 54 deletions
This file was deleted.

‎scripts/jest/config.build-devtools.js

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,6 @@ const packages = readdirSync(packagesRoot).filter(dir => {
2626
// Create a module map to point React packages to the build output
2727
const moduleNameMapper = {};
2828

29-
// Allow bundle tests to read (but not write!) default feature flags.
30-
// This lets us determine whether we're running in different modes
31-
// without making relevant tests internal-only.
32-
moduleNameMapper[
33-
'^shared/ReactFeatureFlags'
34-
] = `<rootDir>/packages/shared/forks/ReactFeatureFlags.readonly`;
35-
3629
// Map packages to bundles
3730
packages.forEach(name => {
3831
// Root entry point
@@ -43,6 +36,10 @@ packages.forEach(name => {
4336
] = `<rootDir>/build/node_modules/${name}/$1`;
4437
});
4538

39+
// Allow tests to import shared code (e.g. feature flags, getStackByFiberInDevAndProd)
40+
moduleNameMapper['^shared\/([^\/]+)$'] = '<rootDir>/packages/shared/$1';
41+
moduleNameMapper['^react-reconciler\/([^\/]+)$'] = '<rootDir>/packages/react-reconciler/$1';
42+
4643
module.exports = Object.assign({}, baseConfig, {
4744
// Redirect imports to the compiled bundles
4845
moduleNameMapper,

0 commit comments

Comments
 (0)