Skip to content

Commit 7b1bded

Browse files
author
Brian Vaughn
committed
Add temporary support for override* commands with mismatched backend/frontend versions
Add message forwarding for older backend methods (overrideContext, overrideHookState, overrideProps, and overrideState) to the new overrideValueAtPath method. This was done in both the frontend Bridge (for newer frontends passing messages to older embedded backends) and in the backend Agent (for older frontends passing messages to newer backends). We do this because React Native embeds the React DevTools backend, but cannot control which version of the frontend users use. Additional unit tests have been added as well to cover the older frontend to newer backend case. Our DevTools test infra does not make it easy to write tests for the other way around.
1 parent 46b33e6 commit 7b1bded

File tree

3 files changed

+304
-0
lines changed

3 files changed

+304
-0
lines changed

packages/react-devtools-shared/src/__tests__/editing-test.js

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,39 @@ describe('editable props and state', () => {
162162
});
163163
});
164164

165+
it('should still support overriding props values with legacy backend methods', async () => {
166+
await mountTestApp();
167+
168+
function overrideProps(id, path, value) {
169+
const rendererID = utils.getRendererID();
170+
bridge.send('overrideProps', {
171+
id,
172+
path,
173+
rendererID,
174+
value,
175+
});
176+
jest.runOnlyPendingTimers();
177+
}
178+
179+
overrideProps(classID, ['object', 'nested'], 'updated');
180+
expect(committedClassProps).toStrictEqual({
181+
array: [1, 2, 3],
182+
object: {
183+
nested: 'updated',
184+
},
185+
shallow: 'initial',
186+
});
187+
188+
overrideProps(functionID, ['shallow'], 'updated');
189+
expect(committedFunctionProps).toStrictEqual({
190+
array: [1, 2, 3],
191+
object: {
192+
nested: 'initial',
193+
},
194+
shallow: 'updated',
195+
});
196+
});
197+
165198
it('should have editable paths', async () => {
166199
await mountTestApp();
167200

@@ -424,6 +457,28 @@ describe('editable props and state', () => {
424457
});
425458
});
426459

460+
it('should still support overriding state values with legacy backend methods', async () => {
461+
await mountTestApp();
462+
463+
function overrideState(path, value) {
464+
const rendererID = utils.getRendererID();
465+
bridge.send('overrideState', {
466+
id,
467+
path,
468+
rendererID,
469+
value,
470+
});
471+
jest.runOnlyPendingTimers();
472+
}
473+
474+
overrideState(['array', 1], 'updated');
475+
expect(committedState).toStrictEqual({
476+
array: [1, 'updated', 3],
477+
object: {nested: 'initial'},
478+
shallow: 'initial',
479+
});
480+
});
481+
427482
it('should have editable paths', async () => {
428483
await mountTestApp();
429484

@@ -623,6 +678,31 @@ describe('editable props and state', () => {
623678
});
624679
});
625680

681+
it('should still support overriding hooks values with legacy backend methods', async () => {
682+
await mountTestApp();
683+
684+
function overrideHookState(path, value) {
685+
const rendererID = utils.getRendererID();
686+
bridge.send('overrideHookState', {
687+
hookID,
688+
id,
689+
path,
690+
rendererID,
691+
value,
692+
});
693+
jest.runOnlyPendingTimers();
694+
}
695+
696+
overrideHookState(['shallow'], 'updated');
697+
expect(committedState).toStrictEqual({
698+
array: [1, 2, 3],
699+
object: {
700+
nested: 'initial',
701+
},
702+
shallow: 'updated',
703+
});
704+
});
705+
626706
it('should have editable paths', async () => {
627707
await mountTestApp();
628708

@@ -858,6 +938,35 @@ describe('editable props and state', () => {
858938
});
859939
});
860940

941+
it('should still support overriding context values with legacy backend methods', async () => {
942+
await mountTestApp();
943+
944+
function overrideContext(path, value) {
945+
const rendererID = utils.getRendererID();
946+
947+
// To simplify hydration and display of primitive context values (e.g. number, string)
948+
// the inspectElement() method wraps context in a {value: ...} object.
949+
path = ['value', ...path];
950+
951+
bridge.send('overrideContext', {
952+
id,
953+
path,
954+
rendererID,
955+
value,
956+
});
957+
jest.runOnlyPendingTimers();
958+
}
959+
960+
overrideContext(['object', 'nested'], 'updated');
961+
expect(committedContext).toStrictEqual({
962+
array: [1, 2, 3],
963+
object: {
964+
nested: 'updated',
965+
},
966+
shallow: 'initial',
967+
});
968+
});
969+
861970
it('should have editable paths', async () => {
862971
await mountTestApp();
863972

packages/react-devtools-shared/src/backend/agent.js

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,23 @@ type InspectElementParams = {|
7474
rendererID: number,
7575
|};
7676

77+
type OverrideHookParams = {|
78+
id: number,
79+
hookID: number,
80+
path: Array<string | number>,
81+
rendererID: number,
82+
wasForwarded?: boolean,
83+
value: any,
84+
|};
85+
86+
type SetInParams = {|
87+
id: number,
88+
path: Array<string | number>,
89+
rendererID: number,
90+
wasForwarded?: boolean,
91+
value: any,
92+
|};
93+
7794
type PathType = 'props' | 'hooks' | 'state' | 'context';
7895

7996
type DeletePathParams = {|
@@ -180,6 +197,14 @@ export default class Agent extends EventEmitter<{|
180197
bridge.addListener('viewAttributeSource', this.viewAttributeSource);
181198
bridge.addListener('viewElementSource', this.viewElementSource);
182199

200+
// Temporarily support older standalone front-ends sending commands to newer embedded backends.
201+
// We do this because React Native embeds the React DevTools backend,
202+
// but cannot control which version of the frontend users use.
203+
bridge.addListener('overrideContext', this.overrideContext);
204+
bridge.addListener('overrideHookState', this.overrideHookState);
205+
bridge.addListener('overrideProps', this.overrideProps);
206+
bridge.addListener('overrideState', this.overrideState);
207+
183208
if (this._isProfiling) {
184209
bridge.send('profilingStatus', true);
185210
}
@@ -338,6 +363,95 @@ export default class Agent extends EventEmitter<{|
338363
}
339364
};
340365

366+
// Temporarily support older standalone front-ends by forwarding the older message types
367+
// to the new "overrideValueAtPath" command the backend is now listening to.
368+
overrideContext = ({
369+
id,
370+
path,
371+
rendererID,
372+
wasForwarded,
373+
value,
374+
}: SetInParams) => {
375+
// Don't forward a message that's already been forwarded by the front-end Bridge.
376+
// We only need to process the override command once!
377+
if (!wasForwarded) {
378+
this.overrideValueAtPath({
379+
id,
380+
path,
381+
rendererID,
382+
type: 'context',
383+
value,
384+
});
385+
}
386+
};
387+
388+
// Temporarily support older standalone front-ends by forwarding the older message types
389+
// to the new "overrideValueAtPath" command the backend is now listening to.
390+
overrideHookState = ({
391+
id,
392+
hookID,
393+
path,
394+
rendererID,
395+
wasForwarded,
396+
value,
397+
}: OverrideHookParams) => {
398+
// Don't forward a message that's already been forwarded by the front-end Bridge.
399+
// We only need to process the override command once!
400+
if (!wasForwarded) {
401+
this.overrideValueAtPath({
402+
id,
403+
path,
404+
rendererID,
405+
type: 'hooks',
406+
value,
407+
});
408+
}
409+
};
410+
411+
// Temporarily support older standalone front-ends by forwarding the older message types
412+
// to the new "overrideValueAtPath" command the backend is now listening to.
413+
overrideProps = ({
414+
id,
415+
path,
416+
rendererID,
417+
wasForwarded,
418+
value,
419+
}: SetInParams) => {
420+
// Don't forward a message that's already been forwarded by the front-end Bridge.
421+
// We only need to process the override command once!
422+
if (!wasForwarded) {
423+
this.overrideValueAtPath({
424+
id,
425+
path,
426+
rendererID,
427+
type: 'props',
428+
value,
429+
});
430+
}
431+
};
432+
433+
// Temporarily support older standalone front-ends by forwarding the older message types
434+
// to the new "overrideValueAtPath" command the backend is now listening to.
435+
overrideState = ({
436+
id,
437+
path,
438+
rendererID,
439+
wasForwarded,
440+
value,
441+
}: SetInParams) => {
442+
// Don't forward a message that's already been forwarded by the front-end Bridge.
443+
// We only need to process the override command once!
444+
if (!wasForwarded) {
445+
this.overrideValueAtPath({
446+
id,
447+
path,
448+
rendererID,
449+
type: 'state',
450+
value,
451+
});
452+
}
453+
};
454+
341455
reloadAndProfile = (recordChangeDescriptions: boolean) => {
342456
sessionStorageSetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, 'true');
343457
sessionStorageSetItem(

packages/react-devtools-shared/src/bridge.js

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,18 @@ type HighlightElementInDOM = {|
3535
scrollIntoView: boolean,
3636
|};
3737

38+
type OverrideValue = {|
39+
...ElementAndRendererID,
40+
path: Array<string | number>,
41+
wasForwarded?: boolean,
42+
value: any,
43+
|};
44+
45+
type OverrideHookState = {|
46+
...OverrideValue,
47+
hookID: number,
48+
|};
49+
3850
type PathType = 'props' | 'hooks' | 'state' | 'context';
3951

4052
type DeletePath = {|
@@ -160,6 +172,21 @@ type FrontendEvents = {|
160172
NativeStyleEditor_measure: [ElementAndRendererID],
161173
NativeStyleEditor_renameAttribute: [NativeStyleEditor_RenameAttributeParams],
162174
NativeStyleEditor_setValue: [NativeStyleEditor_SetValueParams],
175+
176+
// Temporarily support newer standalone front-ends sending commands to older embedded backends.
177+
// We do this because React Native embeds the React DevTools backend,
178+
// but cannot control which version of the frontend users use.
179+
//
180+
// Note that nothing in the newer backend actually listens to these events,
181+
// but the new frontend still dispatches them (in case older backends are listening to them instead).
182+
//
183+
// Note that this approach does no support the combination of a newer backend with an older frontend.
184+
// It would be more work to suppot both approaches (and not run handlers twice)
185+
// so I chose to support the more likely/common scenario (and the one more difficult for an end user to "fix").
186+
overrideContext: [OverrideValue],
187+
overrideHookState: [OverrideHookState],
188+
overrideProps: [OverrideValue],
189+
overrideState: [OverrideValue],
163190
|};
164191

165192
class Bridge<
@@ -184,6 +211,11 @@ class Bridge<
184211
wall.listen((message: Message) => {
185212
(this: any).emit(message.event, message.payload);
186213
}) || null;
214+
215+
// Temporarily support older standalone front-ends sending commands to newer embedded backends.
216+
// We do this because React Native embeds the React DevTools backend,
217+
// but cannot control which version of the frontend users use.
218+
this.addListener('overrideValueAtPath', this.overrideValueAtPath);
187219
}
188220

189221
// Listening directly to the wall isn't advised.
@@ -280,6 +312,55 @@ class Bridge<
280312
this._timeoutID = setTimeout(this._flush, BATCH_DURATION);
281313
}
282314
};
315+
316+
// Temporarily support older standalone backends by forwarding "overrideValueAtPath" commands
317+
// to the older message types they may be listening to.
318+
overrideValueAtPath = ({
319+
id,
320+
path,
321+
rendererID,
322+
type,
323+
value,
324+
}: OverrideValueAtPath) => {
325+
switch (type) {
326+
case 'context':
327+
this.send('overrideContext', {
328+
id,
329+
path,
330+
rendererID,
331+
wasForwarded: true,
332+
value,
333+
});
334+
break;
335+
case 'hooks':
336+
this.send('overrideHookState', {
337+
id,
338+
path,
339+
rendererID,
340+
wasForwarded: true,
341+
value,
342+
});
343+
break;
344+
case 'props':
345+
this.send('overrideProps', {
346+
id,
347+
path,
348+
rendererID,
349+
wasForwarded: true,
350+
value,
351+
});
352+
break;
353+
case 'state':
354+
this.send('overrideState', {
355+
id,
356+
path,
357+
rendererID,
358+
wasForwarded: true,
359+
value,
360+
});
361+
break;
362+
}
363+
};
283364
}
284365

285366
export type BackendBridge = Bridge<BackendEvents, FrontendEvents>;

0 commit comments

Comments
 (0)