Skip to content

Commit

Permalink
Add temporary support for override* commands with mismatched backend/…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
Brian Vaughn committed Sep 17, 2020
1 parent 46b33e6 commit 7b1bded
Show file tree
Hide file tree
Showing 3 changed files with 304 additions and 0 deletions.
109 changes: 109 additions & 0 deletions packages/react-devtools-shared/src/__tests__/editing-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,39 @@ describe('editable props and state', () => {
});
});

it('should still support overriding props values with legacy backend methods', async () => {
await mountTestApp();

function overrideProps(id, path, value) {
const rendererID = utils.getRendererID();
bridge.send('overrideProps', {
id,
path,
rendererID,
value,
});
jest.runOnlyPendingTimers();
}

overrideProps(classID, ['object', 'nested'], 'updated');
expect(committedClassProps).toStrictEqual({
array: [1, 2, 3],
object: {
nested: 'updated',
},
shallow: 'initial',
});

overrideProps(functionID, ['shallow'], 'updated');
expect(committedFunctionProps).toStrictEqual({
array: [1, 2, 3],
object: {
nested: 'initial',
},
shallow: 'updated',
});
});

it('should have editable paths', async () => {
await mountTestApp();

Expand Down Expand Up @@ -424,6 +457,28 @@ describe('editable props and state', () => {
});
});

it('should still support overriding state values with legacy backend methods', async () => {
await mountTestApp();

function overrideState(path, value) {
const rendererID = utils.getRendererID();
bridge.send('overrideState', {
id,
path,
rendererID,
value,
});
jest.runOnlyPendingTimers();
}

overrideState(['array', 1], 'updated');
expect(committedState).toStrictEqual({
array: [1, 'updated', 3],
object: {nested: 'initial'},
shallow: 'initial',
});
});

it('should have editable paths', async () => {
await mountTestApp();

Expand Down Expand Up @@ -623,6 +678,31 @@ describe('editable props and state', () => {
});
});

it('should still support overriding hooks values with legacy backend methods', async () => {
await mountTestApp();

function overrideHookState(path, value) {
const rendererID = utils.getRendererID();
bridge.send('overrideHookState', {
hookID,
id,
path,
rendererID,
value,
});
jest.runOnlyPendingTimers();
}

overrideHookState(['shallow'], 'updated');
expect(committedState).toStrictEqual({
array: [1, 2, 3],
object: {
nested: 'initial',
},
shallow: 'updated',
});
});

it('should have editable paths', async () => {
await mountTestApp();

Expand Down Expand Up @@ -858,6 +938,35 @@ describe('editable props and state', () => {
});
});

it('should still support overriding context values with legacy backend methods', async () => {
await mountTestApp();

function overrideContext(path, value) {
const rendererID = utils.getRendererID();

// To simplify hydration and display of primitive context values (e.g. number, string)
// the inspectElement() method wraps context in a {value: ...} object.
path = ['value', ...path];

bridge.send('overrideContext', {
id,
path,
rendererID,
value,
});
jest.runOnlyPendingTimers();
}

overrideContext(['object', 'nested'], 'updated');
expect(committedContext).toStrictEqual({
array: [1, 2, 3],
object: {
nested: 'updated',
},
shallow: 'initial',
});
});

it('should have editable paths', async () => {
await mountTestApp();

Expand Down
114 changes: 114 additions & 0 deletions packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,23 @@ type InspectElementParams = {|
rendererID: number,
|};

type OverrideHookParams = {|
id: number,
hookID: number,
path: Array<string | number>,
rendererID: number,
wasForwarded?: boolean,
value: any,
|};

type SetInParams = {|
id: number,
path: Array<string | number>,
rendererID: number,
wasForwarded?: boolean,
value: any,
|};

type PathType = 'props' | 'hooks' | 'state' | 'context';

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

// Temporarily support older standalone front-ends sending commands to newer embedded backends.
// We do this because React Native embeds the React DevTools backend,
// but cannot control which version of the frontend users use.
bridge.addListener('overrideContext', this.overrideContext);
bridge.addListener('overrideHookState', this.overrideHookState);
bridge.addListener('overrideProps', this.overrideProps);
bridge.addListener('overrideState', this.overrideState);

if (this._isProfiling) {
bridge.send('profilingStatus', true);
}
Expand Down Expand Up @@ -338,6 +363,95 @@ export default class Agent extends EventEmitter<{|
}
};

// Temporarily support older standalone front-ends by forwarding the older message types
// to the new "overrideValueAtPath" command the backend is now listening to.
overrideContext = ({
id,
path,
rendererID,
wasForwarded,
value,
}: SetInParams) => {
// Don't forward a message that's already been forwarded by the front-end Bridge.
// We only need to process the override command once!
if (!wasForwarded) {
this.overrideValueAtPath({
id,
path,
rendererID,
type: 'context',
value,
});
}
};

// Temporarily support older standalone front-ends by forwarding the older message types
// to the new "overrideValueAtPath" command the backend is now listening to.
overrideHookState = ({
id,
hookID,
path,
rendererID,
wasForwarded,
value,
}: OverrideHookParams) => {
// Don't forward a message that's already been forwarded by the front-end Bridge.
// We only need to process the override command once!
if (!wasForwarded) {
this.overrideValueAtPath({
id,
path,
rendererID,
type: 'hooks',
value,
});
}
};

// Temporarily support older standalone front-ends by forwarding the older message types
// to the new "overrideValueAtPath" command the backend is now listening to.
overrideProps = ({
id,
path,
rendererID,
wasForwarded,
value,
}: SetInParams) => {
// Don't forward a message that's already been forwarded by the front-end Bridge.
// We only need to process the override command once!
if (!wasForwarded) {
this.overrideValueAtPath({
id,
path,
rendererID,
type: 'props',
value,
});
}
};

// Temporarily support older standalone front-ends by forwarding the older message types
// to the new "overrideValueAtPath" command the backend is now listening to.
overrideState = ({
id,
path,
rendererID,
wasForwarded,
value,
}: SetInParams) => {
// Don't forward a message that's already been forwarded by the front-end Bridge.
// We only need to process the override command once!
if (!wasForwarded) {
this.overrideValueAtPath({
id,
path,
rendererID,
type: 'state',
value,
});
}
};

reloadAndProfile = (recordChangeDescriptions: boolean) => {
sessionStorageSetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, 'true');
sessionStorageSetItem(
Expand Down
81 changes: 81 additions & 0 deletions packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ type HighlightElementInDOM = {|
scrollIntoView: boolean,
|};

type OverrideValue = {|
...ElementAndRendererID,
path: Array<string | number>,
wasForwarded?: boolean,
value: any,
|};

type OverrideHookState = {|
...OverrideValue,
hookID: number,
|};

type PathType = 'props' | 'hooks' | 'state' | 'context';

type DeletePath = {|
Expand Down Expand Up @@ -160,6 +172,21 @@ type FrontendEvents = {|
NativeStyleEditor_measure: [ElementAndRendererID],
NativeStyleEditor_renameAttribute: [NativeStyleEditor_RenameAttributeParams],
NativeStyleEditor_setValue: [NativeStyleEditor_SetValueParams],

// Temporarily support newer standalone front-ends sending commands to older embedded backends.
// We do this because React Native embeds the React DevTools backend,
// but cannot control which version of the frontend users use.
//
// Note that nothing in the newer backend actually listens to these events,
// but the new frontend still dispatches them (in case older backends are listening to them instead).
//
// Note that this approach does no support the combination of a newer backend with an older frontend.
// It would be more work to suppot both approaches (and not run handlers twice)
// so I chose to support the more likely/common scenario (and the one more difficult for an end user to "fix").
overrideContext: [OverrideValue],
overrideHookState: [OverrideHookState],
overrideProps: [OverrideValue],
overrideState: [OverrideValue],
|};

class Bridge<
Expand All @@ -184,6 +211,11 @@ class Bridge<
wall.listen((message: Message) => {
(this: any).emit(message.event, message.payload);
}) || null;

// Temporarily support older standalone front-ends sending commands to newer embedded backends.
// We do this because React Native embeds the React DevTools backend,
// but cannot control which version of the frontend users use.
this.addListener('overrideValueAtPath', this.overrideValueAtPath);
}

// Listening directly to the wall isn't advised.
Expand Down Expand Up @@ -280,6 +312,55 @@ class Bridge<
this._timeoutID = setTimeout(this._flush, BATCH_DURATION);
}
};

// Temporarily support older standalone backends by forwarding "overrideValueAtPath" commands
// to the older message types they may be listening to.
overrideValueAtPath = ({
id,
path,
rendererID,
type,
value,
}: OverrideValueAtPath) => {
switch (type) {
case 'context':
this.send('overrideContext', {
id,
path,
rendererID,
wasForwarded: true,
value,
});
break;
case 'hooks':
this.send('overrideHookState', {
id,
path,
rendererID,
wasForwarded: true,
value,
});
break;
case 'props':
this.send('overrideProps', {
id,
path,
rendererID,
wasForwarded: true,
value,
});
break;
case 'state':
this.send('overrideState', {
id,
path,
rendererID,
wasForwarded: true,
value,
});
break;
}
};
}

export type BackendBridge = Bridge<BackendEvents, FrontendEvents>;
Expand Down

0 comments on commit 7b1bded

Please sign in to comment.