Skip to content

Commit

Permalink
Merge branch 'master' of github.com:facebook/react into unacted-effec…
Browse files Browse the repository at this point in the history
…ts-warn
  • Loading branch information
threepointone committed Jun 14, 2019
2 parents 2a6974e + de7a09c commit 1757db6
Show file tree
Hide file tree
Showing 16 changed files with 1,099 additions and 1,177 deletions.
6 changes: 2 additions & 4 deletions .circleci/config.yml
Expand Up @@ -204,10 +204,8 @@ jobs:
- run:
name: Run fuzz tests
command: |
FUZZ_TEST_SEED=$RANDOM
echo $FUZZ_TEST_SEED
yarn test fuzz --maxWorkers=2
yarn test-prod fuzz --maxWorkers=2
FUZZ_TEST_SEED=$RANDOM yarn test fuzz --maxWorkers=2
FUZZ_TEST_SEED=$RANDOM yarn test-prod fuzz --maxWorkers=2
test_build_prod:
docker: *docker
Expand Down
Expand Up @@ -10,7 +10,6 @@
'use strict';

const React = require('react');
const Fragment = React.Fragment;
let ReactFeatureFlags = require('shared/ReactFeatureFlags');

let ReactDOM;
Expand Down Expand Up @@ -602,84 +601,4 @@ describe('ReactDOMFiberAsync', () => {
expect(root.createBatch).toBe(undefined);
});
});

describe('Disable yielding', () => {
beforeEach(() => {
jest.resetModules();
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.disableYielding = true;
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
ReactDOM = require('react-dom');
Scheduler = require('scheduler');
});

it('wont yield during a render if yielding is disabled', () => {
class A extends React.Component {
render() {
Scheduler.yieldValue('A');
return <div>{this.props.children}</div>;
}
}

class B extends React.Component {
render() {
Scheduler.yieldValue('B');
return <div>{this.props.children}</div>;
}
}

class C extends React.Component {
render() {
Scheduler.yieldValue('C');
return <div>{this.props.children}</div>;
}
}

let root = ReactDOM.unstable_createRoot(container);

root.render(
<Fragment>
<A />
<B />
<C />
</Fragment>,
);

expect(Scheduler).toHaveYielded([]);

Scheduler.unstable_flushNumberOfYields(2);
// Even though we just flushed two yields, we should have rendered
// everything without yielding when the flag is on.
expect(Scheduler).toHaveYielded(['A', 'B', 'C']);
});

it('wont suspend during a render if yielding is disabled', () => {
let p = new Promise(resolve => {});

function Suspend() {
throw p;
}

let root = ReactDOM.unstable_createRoot(container);
root.render(
<React.Suspense fallback={'Loading'}>Initial</React.Suspense>,
);

Scheduler.flushAll();
expect(container.textContent).toBe('Initial');

root.render(
<React.Suspense fallback={'Loading'}>
<Suspend />
</React.Suspense>,
);

expect(Scheduler).toHaveYielded([]);

Scheduler.flushAll();

// This should have flushed to the DOM even though we haven't ran the timers.
expect(container.textContent).toBe('Loading');
});
});
});
Expand Up @@ -83,25 +83,25 @@ describe('ReactDOMSuspensePlaceholder', () => {
<div ref={divs[1]}>
<AsyncText ms={500} text="B" />
</div>
<div style={{display: 'block'}} ref={divs[2]}>
<div style={{display: 'inline'}} ref={divs[2]}>
<Text text="C" />
</div>
</Suspense>
);
}
ReactDOM.render(<App />, container);
expect(divs[0].current.style.display).toEqual('none !important');
expect(divs[1].current.style.display).toEqual('none !important');
expect(divs[2].current.style.display).toEqual('none !important');
expect(window.getComputedStyle(divs[0].current).display).toEqual('none');
expect(window.getComputedStyle(divs[1].current).display).toEqual('none');
expect(window.getComputedStyle(divs[2].current).display).toEqual('none');

await advanceTimers(500);

Scheduler.flushAll();

expect(divs[0].current.style.display).toEqual('');
expect(divs[1].current.style.display).toEqual('');
expect(window.getComputedStyle(divs[0].current).display).toEqual('block');
expect(window.getComputedStyle(divs[1].current).display).toEqual('block');
// This div's display was set with a prop.
expect(divs[2].current.style.display).toEqual('block');
expect(window.getComputedStyle(divs[2].current).display).toEqual('inline');
});

it('hides and unhides timed out text nodes', async () => {
Expand Down Expand Up @@ -156,14 +156,14 @@ describe('ReactDOMSuspensePlaceholder', () => {
ReactDOM.render(<App />, container);
});
expect(container.innerHTML).toEqual(
'<span style="display: none !important;">Sibling</span><span style=' +
'"display: none !important;"></span>Loading...',
'<span style="display: none;">Sibling</span><span style=' +
'"display: none;"></span>Loading...',
);

act(() => setIsVisible(true));
expect(container.innerHTML).toEqual(
'<span style="display: none !important;">Sibling</span><span style=' +
'"display: none !important;"></span>Loading...',
'<span style="display: none;">Sibling</span><span style=' +
'"display: none;"></span>Loading...',
);

await advanceTimers(500);
Expand Down
7 changes: 6 additions & 1 deletion packages/react-dom/src/client/ReactDOMHostConfig.js
Expand Up @@ -589,7 +589,12 @@ export function hideInstance(instance: Instance): void {
// TODO: Does this work for all element types? What about MathML? Should we
// pass host context to this method?
instance = ((instance: any): HTMLElement);
instance.style.display = 'none !important';
const style = instance.style;
if (typeof style.setProperty === 'function') {
style.setProperty('display', 'none', 'important');
} else {
style.display = 'none';
}
}

export function hideTextInstance(textInstance: TextInstance): void {
Expand Down
125 changes: 63 additions & 62 deletions packages/react-fresh/src/ReactFreshBabelPlugin.js
Expand Up @@ -163,36 +163,6 @@ export default function(babel) {
return false;
}

let hookCalls = new WeakMap();

function recordHookCall(functionNode, hookCallPath, hookName) {
if (!hookCalls.has(functionNode)) {
hookCalls.set(functionNode, []);
}
let hookCallsForFn = hookCalls.get(functionNode);
let key = '';
if (hookCallPath.parent.type === 'VariableDeclarator') {
// TODO: if there is no LHS, consider some other heuristic.
key = hookCallPath.parentPath.get('id').getSource();
}

// Some built-in Hooks reset on edits to arguments.
const args = hookCallPath.get('arguments');
if (hookName === 'useState' && args.length > 0) {
// useState second argument is initial state.
key += '(' + args[0].getSource() + ')';
} else if (hookName === 'useReducer' && args.length > 1) {
// useReducer second argument is initial state.
key += '(' + args[1].getSource() + ')';
}

hookCallsForFn.push({
name: hookName,
callee: hookCallPath.node.callee,
key,
});
}

function isBuiltinHook(hookName) {
switch (hookName) {
case 'useState':
Expand Down Expand Up @@ -299,9 +269,64 @@ export default function(babel) {

let seenForRegistration = new WeakSet();
let seenForSignature = new WeakSet();
let seenForHookCalls = new WeakSet();
let seenForOutro = new WeakSet();

let hookCalls = new WeakMap();
const HookCallsVisitor = {
CallExpression(path) {
const node = path.node;
const callee = node.callee;

// Note: this visitor MUST NOT mutate the tree in any way.
// It runs early in a separate traversal and should be very fast.

let name = null;
switch (callee.type) {
case 'Identifier':
name = callee.name;
break;
case 'MemberExpression':
name = callee.property.name;
break;
}
if (name === null || !/^use[A-Z]/.test(name)) {
return;
}
const fnScope = path.scope.getFunctionParent();
if (fnScope === null) {
return;
}

// This is a Hook call. Record it.
const fnNode = fnScope.block;
if (!hookCalls.has(fnNode)) {
hookCalls.set(fnNode, []);
}
let hookCallsForFn = hookCalls.get(fnNode);
let key = '';
if (path.parent.type === 'VariableDeclarator') {
// TODO: if there is no LHS, consider some other heuristic.
key = path.parentPath.get('id').getSource();
}

// Some built-in Hooks reset on edits to arguments.
const args = path.get('arguments');
if (name === 'useState' && args.length > 0) {
// useState second argument is initial state.
key += '(' + args[0].getSource() + ')';
} else if (name === 'useReducer' && args.length > 1) {
// useReducer second argument is initial state.
key += '(' + args[1].getSource() + ')';
}

hookCallsForFn.push({
callee: path.node.callee,
name,
key,
});
},
};

return {
visitor: {
ExportDefaultDeclaration(path) {
Expand Down Expand Up @@ -617,38 +642,14 @@ export default function(babel) {
},
);
},
CallExpression(path) {
const node = path.node;
const callee = node.callee;

let name = null;
switch (callee.type) {
case 'Identifier':
name = callee.name;
break;
case 'MemberExpression':
name = callee.property.name;
break;
}
if (name === null || !/^use[A-Z]/.test(name)) {
return;
}

// Make sure we're not recording the same calls twice.
// This can happen if another Babel plugin replaces parents.
if (seenForHookCalls.has(node)) {
return;
}
seenForHookCalls.add(node);
// Don't mutate the tree above this point.

const fn = path.scope.getFunctionParent();
if (fn === null) {
return;
}
recordHookCall(fn.block, path, name);
},
Program: {
enter(path) {
// This is a separate early visitor because we need to collect Hook calls
// and "const [foo, setFoo] = ..." signatures before the destructuring
// transform mangles them. This extra traversal is not ideal for perf,
// but it's the best we can do until we stop transpiling destructuring.
path.traverse(HookCallsVisitor);
},
exit(path) {
const registrations = registrationsByProgramPath.get(path);
if (registrations === undefined) {
Expand Down
6 changes: 3 additions & 3 deletions packages/react-fresh/src/__tests__/ReactFresh-test.js
Expand Up @@ -1361,7 +1361,7 @@ describe('ReactFresh', () => {
const fallbackChild = container.childNodes[1];
expect(primaryChild.textContent).toBe('Content 1');
expect(primaryChild.style.color).toBe('green');
expect(primaryChild.style.display).toBe('none !important');
expect(primaryChild.style.display).toBe('none');
expect(fallbackChild.textContent).toBe('Fallback 0');
expect(fallbackChild.style.color).toBe('green');
expect(fallbackChild.style.display).toBe('');
Expand All @@ -1375,7 +1375,7 @@ describe('ReactFresh', () => {
expect(container.childNodes[1]).toBe(fallbackChild);
expect(primaryChild.textContent).toBe('Content 1');
expect(primaryChild.style.color).toBe('green');
expect(primaryChild.style.display).toBe('none !important');
expect(primaryChild.style.display).toBe('none');
expect(fallbackChild.textContent).toBe('Fallback 1');
expect(fallbackChild.style.color).toBe('green');
expect(fallbackChild.style.display).toBe('');
Expand All @@ -1399,7 +1399,7 @@ describe('ReactFresh', () => {
expect(container.childNodes[1]).toBe(fallbackChild);
expect(primaryChild.textContent).toBe('Content 1');
expect(primaryChild.style.color).toBe('red');
expect(primaryChild.style.display).toBe('none !important');
expect(primaryChild.style.display).toBe('none');
expect(fallbackChild.textContent).toBe('Fallback 1');
expect(fallbackChild.style.color).toBe('red');
expect(fallbackChild.style.display).toBe('');
Expand Down

0 comments on commit 1757db6

Please sign in to comment.