Skip to content

Commit

Permalink
Addressed PR feedback
Browse files Browse the repository at this point in the history 
Also fixed an incorrect boolean check for read-only vs editable hooks.

Also removed a few redundant props.
  • Loading branch information
Brian Vaughn committed Sep 18, 2020
1 parent 6351d46 commit d3597ca
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 68 deletions.
54 changes: 31 additions & 23 deletions packages/react-devtools-shared/src/__tests__/editing-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,18 @@
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
import type Store from 'react-devtools-shared/src/devtools/store';

describe('editable props and state', () => {
describe('editing interface', () => {
let PropTypes;
let React;
let ReactDOM;
let bridge: FrontendBridge;
let store: Store;
let utils;

const flushPendingUpdates = () => {
jest.runOnlyPendingTimers();
};

beforeEach(() => {
utils = require('./utils');

Expand Down Expand Up @@ -108,7 +112,7 @@ describe('editable props and state', () => {
type: 'props',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideProps(classID, ['shallow'], 'updated');
Expand Down Expand Up @@ -162,7 +166,8 @@ describe('editable props and state', () => {
});
});

it('should still support overriding props values with legacy backend methods', async () => {
// Tests the combination of older frontend (DevTools UI) with newer backend (embedded within a renderer).
it('should still support overriding prop values with legacy backend methods', async () => {
await mountTestApp();

function overrideProps(id, path, value) {
Expand All @@ -173,7 +178,7 @@ describe('editable props and state', () => {
rendererID,
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideProps(classID, ['object', 'nested'], 'updated');
Expand Down Expand Up @@ -207,7 +212,7 @@ describe('editable props and state', () => {
rendererID,
type: 'props',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

renamePath(classID, ['shallow'], ['after']);
Expand Down Expand Up @@ -257,7 +262,7 @@ describe('editable props and state', () => {
type: 'props',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideProps(classID, ['new'], 'value');
Expand Down Expand Up @@ -336,7 +341,7 @@ describe('editable props and state', () => {
rendererID,
type: 'props',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

deletePath(classID, ['shallow']);
Expand Down Expand Up @@ -432,7 +437,7 @@ describe('editable props and state', () => {
type: 'state',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideState(['shallow'], 'updated');
Expand All @@ -457,6 +462,7 @@ describe('editable props and state', () => {
});
});

// Tests the combination of older frontend (DevTools UI) with newer backend (embedded within a renderer).
it('should still support overriding state values with legacy backend methods', async () => {
await mountTestApp();

Expand All @@ -468,7 +474,7 @@ describe('editable props and state', () => {
rendererID,
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideState(['array', 1], 'updated');
Expand All @@ -491,7 +497,7 @@ describe('editable props and state', () => {
rendererID,
type: 'state',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

renamePath(['shallow'], ['after']);
Expand Down Expand Up @@ -525,7 +531,7 @@ describe('editable props and state', () => {
type: 'state',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideState(['new'], 'value');
Expand Down Expand Up @@ -572,7 +578,7 @@ describe('editable props and state', () => {
rendererID,
type: 'state',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

deletePath(['shallow']);
Expand Down Expand Up @@ -647,7 +653,7 @@ describe('editable props and state', () => {
type: 'hooks',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideHookState(['shallow'], 'updated');
Expand Down Expand Up @@ -678,7 +684,8 @@ describe('editable props and state', () => {
});
});

it('should still support overriding hooks values with legacy backend methods', async () => {
// Tests the combination of older frontend (DevTools UI) with newer backend (embedded within a renderer).
it('should still support overriding hook values with legacy backend methods', async () => {
await mountTestApp();

function overrideHookState(path, value) {
Expand All @@ -690,7 +697,7 @@ describe('editable props and state', () => {
rendererID,
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideHookState(['shallow'], 'updated');
Expand All @@ -716,7 +723,7 @@ describe('editable props and state', () => {
rendererID,
type: 'hooks',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

renamePath(['shallow'], ['after']);
Expand Down Expand Up @@ -751,7 +758,7 @@ describe('editable props and state', () => {
type: 'hooks',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideHookState(['new'], 'value');
Expand Down Expand Up @@ -799,7 +806,7 @@ describe('editable props and state', () => {
rendererID,
type: 'hooks',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

deletePath(['shallow']);
Expand Down Expand Up @@ -907,7 +914,7 @@ describe('editable props and state', () => {
type: 'context',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideContext(['shallow'], 'updated');
Expand Down Expand Up @@ -938,6 +945,7 @@ describe('editable props and state', () => {
});
});

// Tests the combination of older frontend (DevTools UI) with newer backend (embedded within a renderer).
it('should still support overriding context values with legacy backend methods', async () => {
await mountTestApp();

Expand All @@ -954,7 +962,7 @@ describe('editable props and state', () => {
rendererID,
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideContext(['object', 'nested'], 'updated');
Expand Down Expand Up @@ -985,7 +993,7 @@ describe('editable props and state', () => {
rendererID,
type: 'context',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

renamePath(['shallow'], ['after']);
Expand Down Expand Up @@ -1024,7 +1032,7 @@ describe('editable props and state', () => {
type: 'context',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideContext(['new'], 'value');
Expand Down Expand Up @@ -1076,7 +1084,7 @@ describe('editable props and state', () => {
rendererID,
type: 'context',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

deletePath(['shallow']);
Expand Down
30 changes: 17 additions & 13 deletions packages/react-devtools-shared/src/__tests__/legacy/editing-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
import type Store from 'react-devtools-shared/src/devtools/store';

describe('editable props and state', () => {
describe('editing interface', () => {
let PropTypes;
let React;
let ReactDOM;
Expand All @@ -23,6 +23,10 @@ describe('editable props and state', () => {
jest.runAllTimers(); // Flush Bridge operations
};

const flushPendingUpdates = () => {
jest.runOnlyPendingTimers();
};

beforeEach(() => {
bridge = global.bridge;
store = global.store;
Expand Down Expand Up @@ -92,7 +96,7 @@ describe('editable props and state', () => {
type: 'props',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideProps(['shallow'], 'updated');
Expand Down Expand Up @@ -133,7 +137,7 @@ describe('editable props and state', () => {
rendererID,
type: 'props',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

renamePath(['shallow'], ['after']);
Expand Down Expand Up @@ -166,7 +170,7 @@ describe('editable props and state', () => {
type: 'props',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideProps(['new'], 'value');
Expand Down Expand Up @@ -213,7 +217,7 @@ describe('editable props and state', () => {
rendererID,
type: 'props',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

deletePath(['shallow']);
Expand Down Expand Up @@ -290,7 +294,7 @@ describe('editable props and state', () => {
type: 'state',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideState(['shallow'], 'updated');
Expand Down Expand Up @@ -327,7 +331,7 @@ describe('editable props and state', () => {
rendererID,
type: 'state',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

renamePath(['shallow'], ['after']);
Expand Down Expand Up @@ -361,7 +365,7 @@ describe('editable props and state', () => {
type: 'state',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideState(['new'], 'value');
Expand Down Expand Up @@ -408,7 +412,7 @@ describe('editable props and state', () => {
rendererID,
type: 'state',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

deletePath(['shallow']);
Expand Down Expand Up @@ -511,7 +515,7 @@ describe('editable props and state', () => {
type: 'context',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideContext(['shallow'], 'updated');
Expand Down Expand Up @@ -555,7 +559,7 @@ describe('editable props and state', () => {
rendererID,
type: 'context',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

renamePath(['shallow'], ['after']);
Expand Down Expand Up @@ -590,7 +594,7 @@ describe('editable props and state', () => {
type: 'context',
value,
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

overrideContext(['new'], 'value');
Expand Down Expand Up @@ -638,7 +642,7 @@ describe('editable props and state', () => {
rendererID,
type: 'context',
});
jest.runOnlyPendingTimers();
flushPendingUpdates();
}

deletePath(['shallow']);
Expand Down
Loading

0 comments on commit d3597ca

Please sign in to comment.