Skip to content

Commit ad53a6b

Browse files
authored
trial using context for renderingTarget (#8704)
1 parent 6c01486 commit ad53a6b

58 files changed

Lines changed: 444 additions & 283 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

dotcom-rendering/.eslintrc.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ module.exports = {
108108
ignoreNonDOM: true,
109109
},
110110
],
111+
// We want to be careful with context and certainly avoid unnecessary re-renders
112+
'react/jsx-no-constructed-context-values': 'error',
111113

112114
'@typescript-eslint/switch-exhaustiveness-check': 'error',
113115
'array-callback-return': 'error',
@@ -171,6 +173,10 @@ module.exports = {
171173
},
172174
settings: {
173175
'import/resolver': 'typescript',
176+
react: {
177+
// Tells eslint-plugin-react to automatically detect the version of React to use
178+
version: 'detect',
179+
},
174180
},
175181
overrides: [
176182
{
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { ConfigProvider } from '../../src/components/ConfigContext';
2+
3+
const defaultConfig = { renderingTarget: 'Web' };
4+
5+
export const ConfigContextDecorator = (Story, { args: { config } }) => {
6+
const context = { ...defaultConfig, ...config };
7+
8+
// For easy debugging
9+
console.log('Storybook application config: \n', context);
10+
11+
return (
12+
<ConfigProvider value={context}>
13+
<Story />
14+
</ConfigProvider>
15+
);
16+
};

dotcom-rendering/.storybook/preview.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { Lazy } from '../src/components/Lazy';
1212
import { Picture } from '../src/components/Picture';
1313
import { mockRESTCalls } from '../src/lib/mockRESTCalls';
1414
import { setABTests } from '../src/lib/useAB';
15+
import { ConfigContextDecorator } from './decorators/configContextDecorator';
1516

1617
// Prevent components being lazy rendered when we're taking Chromatic snapshots
1718
Lazy.disabled = isChromatic();
@@ -139,6 +140,7 @@ const guardianViewports = {
139140
/** @type {import('@storybook/react').Preview} */
140141
export default {
141142
decorators: [
143+
ConfigContextDecorator,
142144
(Story) => {
143145
storage.local.clear();
144146
return Story();
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# React Context API
2+
3+
## Context
4+
5+
[A decision](dotcom-rendering/docs/architecture/016-react-context-api.md) was made in 2019 to avoid use of the React context API, preferring prop-drilling and avoiding global state.
6+
7+
[This decision was revisited in 2023](https://github.com/guardian/dotcom-rendering/discussions/8696) due to [the introduction of rendering target as a prop](dotcom-rendering/docs/architecture/proposed-adrs/rendering-type.md) which represents a global state, resulting in very heavy prop-drilling throughout the app. This began to complicate pull requests due to so many unrelated changes appearing in the diff since the props needed to be provided at every layer of the app, as well as tightly coupling components unnecessarily.
8+
9+
An RFC was put together [here](https://github.com/guardian/dotcom-rendering/pull/8704) to trial using the React context API for this specific type of global state, which doesn't change throughout the component lifecycle ie. immutable application configuration.
10+
11+
## Decision
12+
13+
The decision to allow use of context more generally rests on the following _"lines in the sand"_:
14+
15+
- Context should be considered **global, static, and immutable** for rendered components in dotcom-rendering.
16+
- It can be thought of as a type of configuration for our application.
17+
- Context should **not be used for values that change between re-renders**, since this adds unnecessary complexity and there are alternative solutions for these cases.
18+
- There is a eslint rule to attempt to prevent this (`react/jsx-no-constructed-context-values`)
19+
- Context should **only be used for React components** (ie. not for JSON responses or non-JSX helper code)
20+
- This is because the React context API will not work outside of React
21+
22+
## Status
23+
24+
Approved

dotcom-rendering/docs/architecture/3rd party technical review/000-technical-review-records.md renamed to dotcom-rendering/docs/architecture/3rd-party-technical-review/000-technical-review-records.md

File renamed without changes.

dotcom-rendering/docs/architecture/3rd party technical review/001-modi-poc.md renamed to dotcom-rendering/docs/architecture/3rd-party-technical-review/001-modi-poc.md

File renamed without changes.

dotcom-rendering/docs/architecture/3rd party technical review/002-ipsos-mori.md renamed to dotcom-rendering/docs/architecture/3rd-party-technical-review/002-ipsos-mori.md

File renamed without changes.

dotcom-rendering/docs/architecture/3rd party technical review/003-gracenote.md renamed to dotcom-rendering/docs/architecture/3rd-party-technical-review/003-gracenote.md

File renamed without changes.

dotcom-rendering/docs/architecture/3rd party technical review/004-urlbox.md renamed to dotcom-rendering/docs/architecture/3rd-party-technical-review/004-urlbox.md

File renamed without changes.

dotcom-rendering/docs/architecture/016-react-context-api.md renamed to dotcom-rendering/docs/architecture/historic-adrs/016-react-context-api.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,4 @@ the react context api to extract the `edition` property to prevent this.
1515

1616
## Status
1717

18-
Approved
18+
Superseded by 028-react-context-api.md

0 commit comments

Comments
 (0)