Skip to content

Commit

Permalink
Merge branch 'master' into devtools-v4-merge
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Aug 20, 2019
2 parents 85fbe3b + efa5dbe commit 833f206
Show file tree
Hide file tree
Showing 52 changed files with 2,490 additions and 3,096 deletions.
126 changes: 114 additions & 12 deletions packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,17 @@ ESLintTester.setDefaultConfig({
},
});

const eslintTester = new ESLintTester();
eslintTester.run('react-hooks', ReactHooksESLintRule, {
// ***************************************************
// For easier local testing, you can add to any case:
// {
// skip: true,
// --or--
// only: true,
// ...
// }
// ***************************************************

const tests = {
valid: [
`
// Valid because components can use hooks.
Expand Down Expand Up @@ -223,21 +232,20 @@ eslintTester.run('react-hooks', ReactHooksESLintRule, {
(class {i() { useState(); }});
`,
`
// Currently valid although we *could* consider these invalid.
// It doesn't make a lot of difference because it would crash early.
// Valid because they're not matching use[A-Z].
fooState();
use();
_use();
useState();
_useState();
use42();
useHook();
use_hook();
React.useState();
`,
`
// Regression test for the popular "history" library
const {createHistory, useBasename} = require('history-2.1.2');
const browserHistory = useBasename(createHistory)({
// This is grey area.
// Currently it's valid (although React.useCallback would fail here).
// We could also get stricter and disallow it, just like we did
// with non-namespace use*() top-level calls.
const History = require('history-2.1.2');
const browserHistory = History.useBasename(History.createHistory)({
basename: '/',
});
`,
Expand Down Expand Up @@ -669,8 +677,63 @@ eslintTester.run('react-hooks', ReactHooksESLintRule, {
conditionalError('useState'),
],
},
{
code: `
// Invalid because it's dangerous and might not warn otherwise.
// This *must* be invalid.
function useHook({ bar }) {
let foo1 = bar && useState();
let foo2 = bar || useState();
let foo3 = bar ?? useState();
}
`,
errors: [
conditionalError('useState'),
conditionalError('useState'),
// TODO: ideally this *should* warn, but ESLint
// doesn't plan full support for ?? until it advances.
// conditionalError('useState'),
],
},
{
code: `
// Invalid because it's dangerous.
// Normally, this would crash, but not if you use inline requires.
// This *must* be invalid.
// It's expected to have some false positives, but arguably
// they are confusing anyway due to the use*() convention
// already being associated with Hooks.
useState();
if (foo) {
const foo = React.useCallback(() => {});
}
useCustomHook();
`,
errors: [
topLevelError('useState'),
topLevelError('React.useCallback'),
topLevelError('useCustomHook'),
],
},
{
code: `
// Technically this is a false positive.
// We *could* make it valid (and it used to be).
//
// However, top-level Hook-like calls can be very dangerous
// in environments with inline requires because they can mask
// the runtime error by accident.
// So we prefer to disallow it despite the false positive.
const {createHistory, useBasename} = require('history-2.1.2');
const browserHistory = useBasename(createHistory)({
basename: '/',
});
`,
errors: [topLevelError('useBasename')],
},
],
});
};

function conditionalError(hook, hasPreviousFinalizer = false) {
return {
Expand Down Expand Up @@ -708,3 +771,42 @@ function genericError(hook) {
'Hook function.',
};
}

function topLevelError(hook) {
return {
message:
`React Hook "${hook}" cannot be called at the top level. React Hooks ` +
'must be called in a React function component or a custom React ' +
'Hook function.',
};
}

// For easier local testing
if (!process.env.CI) {
let only = [];
let skipped = [];
[...tests.valid, ...tests.invalid].forEach(t => {
if (t.skip) {
delete t.skip;
skipped.push(t);
}
if (t.only) {
delete t.only;
only.push(t);
}
});
const predicate = t => {
if (only.length > 0) {
return only.indexOf(t) !== -1;
}
if (skipped.length > 0) {
return skipped.indexOf(t) === -1;
}
return true;
};
tests.valid = tests.valid.filter(predicate);
tests.invalid = tests.invalid.filter(predicate);
}

const eslintTester = new ESLintTester();
eslintTester.run('react-hooks', ReactHooksESLintRule, tests);
6 changes: 5 additions & 1 deletion packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -432,9 +432,13 @@ export default {
'React Hook function.';
context.report({node: hook, message});
} else if (codePathNode.type === 'Program') {
// For now, ignore if it's in top level scope.
// We could warn here but there are false positives related
// configuring libraries like `history`.
const message =
`React Hook "${context.getSource(hook)}" cannot be called ` +
'at the top level. React Hooks must be called in a ' +
'React function component or a custom React Hook function.';
context.report({node: hook, message});
} else {
// Assume in all other cases the user called a hook in some
// random function callback. This should usually be true for
Expand Down
20 changes: 17 additions & 3 deletions packages/legacy-events/ReactGenericBatching.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
} from './ReactControlledComponent';
import {enableFlareAPI} from 'shared/ReactFeatureFlags';

import {invokeGuardedCallbackAndCatchFirstError} from 'shared/ReactErrorUtils';

// Used as a way to call batchedUpdates when we don't have a reference to
// the renderer. Such as when we're dispatching events or if third party
// libraries need to call batchedUpdates. Eventually, this API will go away when
Expand All @@ -28,6 +30,7 @@ let flushDiscreteUpdatesImpl = function() {};
let batchedEventUpdatesImpl = batchedUpdatesImpl;

let isInsideEventHandler = false;
let isBatchingEventUpdates = false;

function finishEventHandler() {
// Here we wait until all updates have propagated, which is important
Expand Down Expand Up @@ -60,20 +63,31 @@ export function batchedUpdates(fn, bookkeeping) {
}

export function batchedEventUpdates(fn, a, b) {
if (isInsideEventHandler) {
if (isBatchingEventUpdates) {
// If we are currently inside another batch, we need to wait until it
// fully completes before restoring state.
return fn(a, b);
}
isInsideEventHandler = true;
isBatchingEventUpdates = true;
try {
return batchedEventUpdatesImpl(fn, a, b);
} finally {
isInsideEventHandler = false;
isBatchingEventUpdates = false;
finishEventHandler();
}
}

export function executeUserEventHandler(fn: any => void, value: any) {
const previouslyInEventHandler = isInsideEventHandler;
try {
isInsideEventHandler = true;
const type = typeof value === 'object' && value !== null ? value.type : '';
invokeGuardedCallbackAndCatchFirstError(type, fn, undefined, value);
} finally {
isInsideEventHandler = previouslyInEventHandler;
}
}

export function discreteUpdates(fn, a, b, c) {
const prevIsInsideEventHandler = isInsideEventHandler;
isInsideEventHandler = true;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-art/src/ReactART.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class Surface extends React.Component {

this._surface = Mode.Surface(+width, +height, this._tagRef);

this._mountNode = createContainer(this._surface, LegacyRoot, false);
this._mountNode = createContainer(this._surface, LegacyRoot, false, null);
updateContainer(this.props.children, this._mountNode, this);
}

Expand Down
46 changes: 38 additions & 8 deletions packages/react-devtools/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,23 +1,47 @@
# React DevTools changelog

<!-- ## [Unreleased]
<details>
<summary>
Changes that have landed in master but are not yet released.
Click to see more.
</summary>

<!-- Upcoming changes go here
</details> -->
<!-- Upcoming changes go here -->
</details>

## 4.0.0 (release date TBD)
## 4.0.5 (August 19, 2019)
#### Bug fixes
* Props, state, and context values are alpha sorted.
* Standalone DevTools properly serves backend script over localhost:8097

## 4.0.4 (August 18, 2019)
#### Bug fixes
* Bugfix for potential error if a min-duration commit filter is applied after selecting a fiber in the Profiler UI.

## 4.0.3 (August 17, 2019)
#### Bug fixes
* ES6 `Map` and `Set`, typed arrays, and other unserializable types (e.g. Immutable JS) can now be inspected.
* Empty objects and arrays now display an "(empty)" label to the right to reduce confusion.
* Components that use only the `useContext` hook now properly display hooks values in side panel.
* Style editor now supports single quotes around string values (e.g. both `"red"` and `'red'`).
* Fixed edge case bug that prevented profiling when both React v16 and v15 were present on a page.

## 4.0.2 (August 15, 2019)
#### Permissions cleanup
* Removed unnecessary `webNavigation ` permission from Chrome and Firefox extensions.

## 4.0.1 (August 15, 2019)
#### Permissions cleanup
* Removed unnecessary `<all_urls>`, `background`, and `tabs` permissions from Chrome and Firefox extensions.

## 4.0.0 (August 15, 2019)

### General changes

#### Improved performance
The legacy DevTools extension used to add significant performance overhead, making it unusable for some larger React applications. That overhead has been effectively eliminated in version 4.

[Learn more](https://github.com/bvaughn/react-devtools-experimental/blob/master/OVERVIEW.md) about the performance optimizations that made this possible.
[Learn more](https://github.com/facebook/react/blob/master/packages/react-devtools/OVERVIEW.md) about the performance optimizations that made this possible.

#### Component stacks

Expand All @@ -41,17 +65,17 @@ Host nodes (e.g. HTML `<div>`, React Native `View`) are now hidden by default, b

Filter preferences are remembered between sessions.

#### No more in-line props
#### No more inline props

Components in the tree no longer show in-line props. This was done to [make DevTools faster](https://github.com/bvaughn/react-devtools-experimental/blob/master/OVERVIEW.md) and to make it easier to browse larger component trees.
Components in the tree no longer show inline props. This was done to [make DevTools faster](https://github.com/facebook/react/blob/master/packages/react-devtools/OVERVIEW.md) and to make it easier to browse larger component trees.

You can view a component's props, state, and hooks by selecting it:

![Inspecting props](https://user-images.githubusercontent.com/29597/62303001-37da6400-b430-11e9-87fd-10a94df88efa.png)

#### "Rendered by" list

In React, an element's "owner" refers the thing that rendered it. Sometimes an element's parent is also its owner, but usually they're different. This distinction is important because props come from owners.
In React, an element's "owner" refers to the thing that rendered it. Sometimes an element's parent is also its owner, but usually they're different. This distinction is important because props come from owners.

![Example code](https://user-images.githubusercontent.com/29597/62229551-bbcf1600-b374-11e9-8411-8ff411f4f847.png)

Expand Down Expand Up @@ -101,6 +125,12 @@ Components decorated with multiple HOCs show the topmost badge and a count. Sele

![Screenshot showing a component with multiple HOC badges](https://user-images.githubusercontent.com/29597/62303729-7fadbb00-b431-11e9-8685-45f5ab52b30b.png)

#### Restoring selection between reloads

DevTools now attempts to restore the previously selected element when you reload the page.

![Video demonstrating selection persistence](https://user-images.githubusercontent.com/810438/63130054-2c02ac00-bfb1-11e9-92fa-382e9e433638.gif)

#### Suspense toggle

React's experimental [Suspense API](https://reactjs.org/docs/react-api.html#suspense) lets components "wait" for something before rendering. `<Suspense>` components can be used to specify loading states when components deeper in the tree are waiting to render.
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools/OVERVIEW.md
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ Even when dealing with a single component, serializing deeply nested properties

Hooks present a unique challenge for the DevTools because of the concept of _custom_ hooks. (A custom hook is essentially any function that calls at least one of the built-in hooks. By convention custom hooks also have names that begin with "use".)

So how does DevTools identify custom functions called from within third party components? It does this by temporarily overriding React's built-in hooks and shallow rendering the component in question. Whenever one of the (overridden) built-in hooks are called, it parses the call stack to spot potential custom hooks (functions between the component itself and the built-in hook). This approach enables it to build a tree structure describing all of the calls to both the built-in _and_ custom hooks, along with the values passed to those hooks. (If you're interested in learning more about this, [here is the source code](https://github.com/bvaughn/react-devtools-experimental/blob/master/src/backend/ReactDebugHooks.js).)
So how does DevTools identify custom functions called from within third party components? It does this by temporarily overriding React's built-in hooks and shallow rendering the component in question. Whenever one of the (overridden) built-in hooks are called, it parses the call stack to spot potential custom hooks (functions between the component itself and the built-in hook). This approach enables it to build a tree structure describing all of the calls to both the built-in _and_ custom hooks, along with the values passed to those hooks. (If you're interested in learning more about this, [here is the source code](https://github.com/facebook/react/blob/master/packages/react-debug-tools/src/ReactDebugHooks.js).)

> **Note**: DevTools obtains hooks info by re-rendering a component.
> Breakpoints will be invoked during this additional (shallow) render,
Expand Down
Loading

0 comments on commit 833f206

Please sign in to comment.