Skip to content

Commit

Permalink
Add test for context propagation compat bug
Browse files Browse the repository at this point in the history
Found a difference between how React and Preact propagate context. It shows up when using react-router and a component triggers a location update (e.g. `history.push`) and at the same time triggers a local state update.

## The setup

First, how React Router works. The `Router` component watches the pages current location and provides it on a context. Let's call this context `RouterContext`. The `Route` component consumes this `RouterContext`, but also modifies it and re-provides the new value on a new instance of `RouterContext.Provider`.

So if we have a `Page` component that lives under a `Router` and `Route` components, the virtual DOM tree looks something the following. Let's say the initial value of the provided location in `RouterContext` is `/page/1`

```jsx
<App>
  <Router>
    <RouterContext.Provider> {/* location: /page/1 */}
      <Route>
        <RouterContext.Consumer>
          <RouterContext.Provider> {/* location: /page/1 */}
            <Page>
              <button>
```

Let's say the `button` under `Page` has a click handler that triggers a location update on `Router` as well as updates some local state:

```jsx
function Page() {
	const history = useHistory(); // Magical hook that triggers a state update in the Router with a new location entry
	const [value, setValue] = useState(1);

	// let's pretend to read the current location and value for some purpose
	const { location } = useContext(RouterContext);
	console.log({ location, value });

	return (
		<button
			onClick={() => {
				history.push('/page/2'); // Triggers an state update in the Router component
				setValue(2); // Triggers a state update in Page
			}}
		>
			Update
		</button>
	);
}
```

Some things to note about this virtual tree:

1. The `RouteContext.Consumer` under `Route` is subscribed to its parent `RouteContext.Provider` (the one owned by `Router`)
2. The `Page` component is subscribed to it's nearest parent `RouteContext.Provider`, which in this case is the one owned by `Route`, not `Router`.

On initial render, `Page` would log `{ location: /page/1, value: 1 }`.

## The bug

So what happens in Preact when we click this button? Two state updates are triggered: one on `Router` and one `Page`. So our rerender queue looks like [`Router`, `Page`]. Upon rerendering Router, we update the location value we pass to the first `RouterContext.Provider` to `/page/2` and trigger updates to all of the `RouterContext` consumers. Let's re-examine our render queue and virtual DOM tree:

```jsx
// renderQueue: [Page, RouterContext.Consumer]
<App>
  <Router>
    <RouterContext.Provider> {/* location: /page/2 */}
      <Route>
        <RouterContext.Consumer>
          <RouterContext.Provider> {/* location: /page/1 */}
            <Page>
              <button>
```

A couple things to note. Our rerendering stops after the first `RouterContext.Provider` runs. Context providers just return `props.children` so our `vnodeID` optimization will kick in, see the children of the `Provider` didn't get no VNodes and stop rerendering there. Because of this, the `RouterContext.Consumer` under `Route` has NOT rerendered yet, so the `RouterContext.Provider` that it owns has not received the updated context. And since the `Page` component is subscribed to the `RouterContext.Provider` that `Route` owns, it still would see the old context.

So, since rerendering stopped after the first `RouterContext.Provider`, the next component that rerenders is `Page` (remember, it triggered it's own local state update). At this point `Page` will log `{ location: /page/1, value: 2 }`. This log represents the developer-observable bug.

The `onClick` handler of the button contains both the `history.push()` and `setValue()` calls and so the developer expects that the next time `Page` renders will be both a new location and a new state value. And logic that assumes this will fail.

## Completing the flow

If we continue with Preact's rendering (for completeness), once the `RouterContext.Consumer` rerenders, it'll rerender the second `RouterContext.Provider` which trigger rerenders to its subscribers, namely `Page`. A this point `Page` will rerender again but with the both the updated location context and the new state and so will log `{ location: /page/2, value: 2 }`.

## React's behavior

In this case, React handles the state propagation and local state update in one pass down the render tree. Presumably, changes to context synchronously mark consumers as needing updates (I haven't personally validated this yet). So as React walks down the tree, it rerenders the `Router`, the first `RouterContext.Provider`, sees the `RouterContext.Consumer` as needing a rerender, which causes a render of the second `RouterContext.Provider`, and ultimately rerenders the `Page` component with both it's local state update and the location update from its parent `RouterContext.Provider`. In this case, `Page` only rerenders once with both updates.
  • Loading branch information
andrewiggins committed Jan 24, 2023
1 parent c483d96 commit 672782a
Showing 1 changed file with 144 additions and 0 deletions.
144 changes: 144 additions & 0 deletions compat/test/browser/context.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import { setupRerender } from 'preact/test-utils';
import { setupScratch, teardown } from '../../../test/_util/helpers';
import React, {
render,
createElement,
createContext,
Component,
useState,
useContext
} from 'preact/compat';

describe('components', () => {
/** @type {HTMLDivElement} */
let scratch;

/** @type {() => void} */
let rerender;

beforeEach(() => {
scratch = setupScratch();
rerender = setupRerender();
});

afterEach(() => {
teardown(scratch);
});

it('nested context updates propagate throughout the tree synchronously', () => {
const RouterContext = createContext({ location: '__default_value__' });

const route1 = '/page/1';
const route2 = '/page/2';

/** @type {() => void} */
let toggleLocalState;
/** @type {() => void} */
let toggleLocation;

/** @type {Array<{location: string, localState: boolean}>} */
let pageRenders = [];

function runUpdate() {
toggleLocalState();
toggleLocation();
}

/**
* @extends {React.Component<{children: any}, {location: string}>}
*/
class Router extends Component {
constructor(props) {
super(props);
this.state = { location: route1 };
toggleLocation = () => {
const oldLocation = this.state.location;
const newLocation = oldLocation === route1 ? route2 : route1;
console.log('Toggling location', oldLocation, '->', newLocation);
this.setState({ location: newLocation });
};
}

render() {
console.log('Rendering Router', { location: this.state.location });
return (
<RouterContext.Provider value={{ location: this.state.location }}>
{this.props.children}
</RouterContext.Provider>
);
}
}

/**
* @extends {React.Component<{children: any}>}
*/
class Route extends Component {
render() {
return (
<RouterContext.Consumer>
{contextValue => {
console.log('Rendering Route', {
location: contextValue.location
});
// Pretend to do something with the context value
const newContextValue = { ...contextValue };
return (
<RouterContext.Provider value={newContextValue}>
{this.props.children}
</RouterContext.Provider>
);
}}
</RouterContext.Consumer>
);
}
}

function Page() {
const [localState, setLocalState] = useState(true);
const { location } = useContext(RouterContext);

pageRenders.push({ location, localState });
console.log('Rendering Page', { location, localState });

toggleLocalState = () => {
let newValue = !localState;
console.log('Toggling localState', localState, '->', newValue);
setLocalState(newValue);
};

return (
<>
<div>localState: {localState.toString()}</div>
<div>location: {location}</div>
<div>
<button type="button" onClick={runUpdate}>
Trigger update
</button>
</div>
</>
);
}

function App() {
return (
<Router>
<Route>
<Page />
</Route>
</Router>
);
}

render(<App />, scratch);
expect(pageRenders).to.deep.equal([{ location: route1, localState: true }]);

pageRenders = [];
runUpdate(); // Simulate button click
rerender();

// Page should rerender once with both propagated context and local state updates
expect(pageRenders).to.deep.equal([
{ location: route2, localState: false }
]);
});
});

0 comments on commit 672782a

Please sign in to comment.