Skip to content

Commit 9e829ac

Browse files
committed
docs/react: Update shouldComponentUpdate / Context API discussion
The substance of this about React, shouldComponentUpdate, and the (new-style) Context API is still valid. But there's no longer a live example of it in our codebase.
1 parent 0122e45 commit 9e829ac

File tree

1 file changed

+9
-6
lines changed

1 file changed

+9
-6
lines changed

docs/architecture/react.md

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,13 @@ doc](https://reactjs.org/docs/context.html)):
141141
> [...])
142142
> is not subject to the shouldComponentUpdate method
143143
144-
Concretely, this means that our `MessageList` component updates
145-
(re-`render`s) when the theme changes, since it's a `ThemeContext`
146-
consumer, *even though its `shouldComponentUpdate` always returns
147-
`false`*. So far, this hasn't been a problem because the UI doesn't
144+
We also confirmed this behavior experimentally, in a 2020 version of
145+
`MessageList` which used `ThemeContext` to get the theme colors.
146+
(Since 1ba871910, `MessageList` uses a transparent background and so
147+
doesn't need the theme; since 4fa2418b8 it doesn't mention
148+
`ThemeContext`.) That component re-`render`ed when the theme changed,
149+
*even though its `shouldComponentUpdate` always returned `false`*.
150+
This didn't cause a live problem because the UI doesn't
148151
allow changing the theme while a `MessageList` is in the navigation
149152
stack. If it were possible, it would be a concern: setting a short
150153
interval to automatically toggle the theme, we see that the message
@@ -167,8 +170,8 @@ it could use `PureComponent`, but it doesn't -- instead we have a
167170
`shouldComponentUpdate` that always returns `false`, so even when `props`
168171
change quite materially (e.g., a new Zulip message arrives which should be
169172
displayed) we don't have React re-render the component. (See the note
170-
on the current Context API, above, for a known case where our
171-
`shouldComponentUpdate` is ignored.)
173+
on the current Context API, above, for a known case where
174+
`shouldComponentUpdate` can be ignored.)
172175

173176
The specifics of why not, and what we do instead, deserve an architecture
174177
doc of their own. In brief: `render` returns a single React element, a

0 commit comments

Comments
 (0)