diff --git a/dotcom-rendering/.eslintrc.js b/dotcom-rendering/.eslintrc.js index 5631f8b294c..2e3d382ebed 100644 --- a/dotcom-rendering/.eslintrc.js +++ b/dotcom-rendering/.eslintrc.js @@ -108,6 +108,8 @@ module.exports = { ignoreNonDOM: true, }, ], + // We want to be careful with context and certainly avoid unnecessary re-renders + 'react/jsx-no-constructed-context-values': 'error', '@typescript-eslint/switch-exhaustiveness-check': 'error', 'array-callback-return': 'error', @@ -171,6 +173,10 @@ module.exports = { }, settings: { 'import/resolver': 'typescript', + react: { + // Tells eslint-plugin-react to automatically detect the version of React to use + version: 'detect', + }, }, overrides: [ { diff --git a/dotcom-rendering/.storybook/decorators/configContextDecorator.js b/dotcom-rendering/.storybook/decorators/configContextDecorator.js new file mode 100644 index 00000000000..f04f24d662b --- /dev/null +++ b/dotcom-rendering/.storybook/decorators/configContextDecorator.js @@ -0,0 +1,16 @@ +import { ConfigProvider } from '../../src/components/ConfigContext'; + +const defaultConfig = { renderingTarget: 'Web' }; + +export const ConfigContextDecorator = (Story, { args: { config } }) => { + const context = { ...defaultConfig, ...config }; + + // For easy debugging + console.log('Storybook application config: \n', context); + + return ( + + + + ); +}; diff --git a/dotcom-rendering/.storybook/preview.js b/dotcom-rendering/.storybook/preview.js index 309fa5fb131..8d78fa8b7bc 100644 --- a/dotcom-rendering/.storybook/preview.js +++ b/dotcom-rendering/.storybook/preview.js @@ -12,6 +12,7 @@ import { Lazy } from '../src/components/Lazy'; import { Picture } from '../src/components/Picture'; import { mockRESTCalls } from '../src/lib/mockRESTCalls'; import { setABTests } from '../src/lib/useAB'; +import { ConfigContextDecorator } from './decorators/configContextDecorator'; // Prevent components being lazy rendered when we're taking Chromatic snapshots Lazy.disabled = isChromatic(); @@ -139,6 +140,7 @@ const guardianViewports = { /** @type {import('@storybook/react').Preview} */ export default { decorators: [ + ConfigContextDecorator, (Story) => { storage.local.clear(); return Story(); diff --git a/dotcom-rendering/docs/architecture/028-react-context-api.md b/dotcom-rendering/docs/architecture/028-react-context-api.md new file mode 100644 index 00000000000..afca5054db9 --- /dev/null +++ b/dotcom-rendering/docs/architecture/028-react-context-api.md @@ -0,0 +1,24 @@ +# React Context API + +## Context + +[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. + +[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. + +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. + +## Decision + +The decision to allow use of context more generally rests on the following _"lines in the sand"_: + +- Context should be considered **global, static, and immutable** for rendered components in dotcom-rendering. + - It can be thought of as a type of configuration for our application. +- 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. + - There is a eslint rule to attempt to prevent this (`react/jsx-no-constructed-context-values`) +- Context should **only be used for React components** (ie. not for JSON responses or non-JSX helper code) + - This is because the React context API will not work outside of React + +## Status + +Approved diff --git a/dotcom-rendering/docs/architecture/3rd party technical review/000-technical-review-records.md b/dotcom-rendering/docs/architecture/3rd-party-technical-review/000-technical-review-records.md similarity index 100% rename from dotcom-rendering/docs/architecture/3rd party technical review/000-technical-review-records.md rename to dotcom-rendering/docs/architecture/3rd-party-technical-review/000-technical-review-records.md diff --git a/dotcom-rendering/docs/architecture/3rd party technical review/001-modi-poc.md b/dotcom-rendering/docs/architecture/3rd-party-technical-review/001-modi-poc.md similarity index 100% rename from dotcom-rendering/docs/architecture/3rd party technical review/001-modi-poc.md rename to dotcom-rendering/docs/architecture/3rd-party-technical-review/001-modi-poc.md diff --git a/dotcom-rendering/docs/architecture/3rd party technical review/002-ipsos-mori.md b/dotcom-rendering/docs/architecture/3rd-party-technical-review/002-ipsos-mori.md similarity index 100% rename from dotcom-rendering/docs/architecture/3rd party technical review/002-ipsos-mori.md rename to dotcom-rendering/docs/architecture/3rd-party-technical-review/002-ipsos-mori.md diff --git a/dotcom-rendering/docs/architecture/3rd party technical review/003-gracenote.md b/dotcom-rendering/docs/architecture/3rd-party-technical-review/003-gracenote.md similarity index 100% rename from dotcom-rendering/docs/architecture/3rd party technical review/003-gracenote.md rename to dotcom-rendering/docs/architecture/3rd-party-technical-review/003-gracenote.md diff --git a/dotcom-rendering/docs/architecture/3rd party technical review/004-urlbox.md b/dotcom-rendering/docs/architecture/3rd-party-technical-review/004-urlbox.md similarity index 100% rename from dotcom-rendering/docs/architecture/3rd party technical review/004-urlbox.md rename to dotcom-rendering/docs/architecture/3rd-party-technical-review/004-urlbox.md diff --git a/dotcom-rendering/docs/architecture/016-react-context-api.md b/dotcom-rendering/docs/architecture/historic-adrs/016-react-context-api.md similarity index 93% rename from dotcom-rendering/docs/architecture/016-react-context-api.md rename to dotcom-rendering/docs/architecture/historic-adrs/016-react-context-api.md index 3fb76ca49b1..c16a59a3af3 100644 --- a/dotcom-rendering/docs/architecture/016-react-context-api.md +++ b/dotcom-rendering/docs/architecture/historic-adrs/016-react-context-api.md @@ -15,4 +15,4 @@ the react context api to extract the `edition` property to prevent this. ## Status -Approved +Superseded by 028-react-context-api.md diff --git a/dotcom-rendering/docs/architecture/historic adrs/dotenv.md b/dotcom-rendering/docs/architecture/historic-adrs/dotenv.md similarity index 100% rename from dotcom-rendering/docs/architecture/historic adrs/dotenv.md rename to dotcom-rendering/docs/architecture/historic-adrs/dotenv.md diff --git a/dotcom-rendering/docs/architecture/historic adrs/multiple-sites.md b/dotcom-rendering/docs/architecture/historic-adrs/multiple-sites.md similarity index 100% rename from dotcom-rendering/docs/architecture/historic adrs/multiple-sites.md rename to dotcom-rendering/docs/architecture/historic-adrs/multiple-sites.md diff --git a/dotcom-rendering/docs/architecture/historic adrs/signing-image-urls.md b/dotcom-rendering/docs/architecture/historic-adrs/signing-image-urls.md similarity index 100% rename from dotcom-rendering/docs/architecture/historic adrs/signing-image-urls.md rename to dotcom-rendering/docs/architecture/historic-adrs/signing-image-urls.md diff --git a/dotcom-rendering/docs/architecture/historic adrs/use-percy-for-e2e-visual-regression-testing.md b/dotcom-rendering/docs/architecture/historic-adrs/use-percy-for-e2e-visual-regression-testing.md similarity index 100% rename from dotcom-rendering/docs/architecture/historic adrs/use-percy-for-e2e-visual-regression-testing.md rename to dotcom-rendering/docs/architecture/historic-adrs/use-percy-for-e2e-visual-regression-testing.md diff --git a/dotcom-rendering/docs/architecture/proposed adrs/rendering-type.md b/dotcom-rendering/docs/architecture/proposed-adrs/rendering-type.md similarity index 100% rename from dotcom-rendering/docs/architecture/proposed adrs/rendering-type.md rename to dotcom-rendering/docs/architecture/proposed-adrs/rendering-type.md diff --git a/dotcom-rendering/docs/development/storybook.md b/dotcom-rendering/docs/development/storybook.md new file mode 100644 index 00000000000..88041adccf1 --- /dev/null +++ b/dotcom-rendering/docs/development/storybook.md @@ -0,0 +1,10 @@ +# Storybook + +We use Storybook to visualise our components in an isolated environment, where we can tweak the conditions as we want. +We use Chromatic for visual regression testing of Storybook components. + +## Rendering context + +We use context for static, global state in the dotcom-rendering app, so every story is wrapped in a context provider component. In the real world, our top component includes this context provider. + +In Storybook is largely invisible as it's hidden within the [configuration](dotcom-rendering/.storybook). There's a decorator configured to wrap around stories and log the context output to the console, for easier debugging. diff --git a/dotcom-rendering/index.d.ts b/dotcom-rendering/index.d.ts index 73d6c55a20f..b10e2982ccd 100644 --- a/dotcom-rendering/index.d.ts +++ b/dotcom-rendering/index.d.ts @@ -521,6 +521,11 @@ declare namespace JSX { clientOnly?: boolean; props: any; children: React.ReactNode; + /** + * This should be a stringified JSON of `ConfigContext` + * @see /dotcom-rendering/src/types/configContext.ts + */ + config: string; }; } diff --git a/dotcom-rendering/scripts/gen-stories/get-stories.js b/dotcom-rendering/scripts/gen-stories/get-stories.js index 45c246776d8..29a6b9768bb 100644 --- a/dotcom-rendering/scripts/gen-stories/get-stories.js +++ b/dotcom-rendering/scripts/gen-stories/get-stories.js @@ -119,6 +119,7 @@ export const ${storyVariableName} = () => { ); }; ${storyVariableName}.storyName = '${renderingTarget}: Display: ${displayName}, Design: ${designName}, Theme: ${theme}'; +${storyVariableName}.args = { config: { renderingTarget: '${renderingTarget}' } }; `; }; diff --git a/dotcom-rendering/scripts/jest/setup.ts b/dotcom-rendering/scripts/jest/setup.ts index 247ca378647..299a0b66617 100644 --- a/dotcom-rendering/scripts/jest/setup.ts +++ b/dotcom-rendering/scripts/jest/setup.ts @@ -113,3 +113,12 @@ global.TextDecoder = TextDecoder as unknown as typeof global.TextDecoder; // Mocks the version number used by CDK, we don't want our tests to fail every time we update our cdk dependency. jest.mock('@guardian/cdk/lib/constants/tracking-tag'); + +// Mocks the useConfig hook in ConfigContext, so that we don't have to use the provider all the time +jest.mock('../../src/components/ConfigContext.tsx', () => { + const mockConfig = { renderingTarget: 'Web' }; + return { + ...jest.requireActual('../../src/components/ConfigContext.tsx'), + useConfig: () => mockConfig, + }; +}); diff --git a/dotcom-rendering/src/client/discussion.ts b/dotcom-rendering/src/client/discussion.ts index e71301193bf..76c1a06201f 100644 --- a/dotcom-rendering/src/client/discussion.ts +++ b/dotcom-rendering/src/client/discussion.ts @@ -1,5 +1,6 @@ import { doHydration } from './islands/doHydration'; import { getEmotionCache } from './islands/emotion'; +import { getConfig } from './islands/getConfig'; import { getProps } from './islands/getProps'; const forceHydration = async (): Promise => { @@ -12,14 +13,15 @@ const forceHydration = async (): Promise => { ); if (!guElement) return; - // Read the props from where they have been serialised in the dom using an Island + // Read the props and config from where they have been serialised in the dom using an Island const props = getProps(guElement); + const config = getConfig(guElement); // Now that we have the props as an object, tell Discussion we want it to expand itself props.expanded = true; // Force hydration - await doHydration(name, props, guElement, getEmotionCache()); + await doHydration(name, props, guElement, getEmotionCache(), config); } catch (err) { // Do nothing } diff --git a/dotcom-rendering/src/client/islands/doHydration.tsx b/dotcom-rendering/src/client/islands/doHydration.tsx index 88abe23a60c..a6d65442565 100644 --- a/dotcom-rendering/src/client/islands/doHydration.tsx +++ b/dotcom-rendering/src/client/islands/doHydration.tsx @@ -4,6 +4,8 @@ import { CacheProvider } from '@emotion/react'; import { log, startPerformanceMeasure } from '@guardian/libs'; import { createElement } from 'react'; import { createRoot, hydrateRoot } from 'react-dom/client'; +import { ConfigProvider } from '../../components/ConfigContext'; +import type { Config } from '../../types/configContext'; declare global { interface DOMStringMap { @@ -26,12 +28,14 @@ declare global { * @param data The deserialised props we want to use for hydration * @param element The location on the DOM where the component to hydrate exists * @param emotionCache An instance of an emotion cache to use for the island + * @param config Application configuration to be passed to the config context for the hydrated component */ export const doHydration = async ( name: string, data: { [key: string]: unknown } | null, element: HTMLElement, emotionCache: EmotionCache, + config: Config, ): Promise => { // If this function has already been run for an element then don't try to // run it a second time @@ -58,16 +62,20 @@ export const doHydration = async ( element.querySelector('[data-name="placeholder"]')?.remove(); const root = createRoot(element); root.render( - - {createElement(module[name], data)} - , + + + {createElement(module[name], data)} + + , ); } else { hydrateRoot( element, - - {createElement(module[name], data)} - , + + + {createElement(module[name], data)} + + , ); } diff --git a/dotcom-rendering/src/client/islands/doStorybookHydration.js b/dotcom-rendering/src/client/islands/doStorybookHydration.js index de3b089d384..f3431fed6ce 100644 --- a/dotcom-rendering/src/client/islands/doStorybookHydration.js +++ b/dotcom-rendering/src/client/islands/doStorybookHydration.js @@ -1,5 +1,7 @@ import { createElement } from 'react'; import { createRoot } from 'react-dom/client'; +import { ConfigProvider } from '../../components/ConfigContext'; +import { getConfig } from './getConfig'; import { getName } from './getName'; import { getProps } from './getProps'; @@ -16,13 +18,14 @@ import { getProps } from './getProps'; * snapshots */ export const doStorybookHydration = () => { - document.querySelectorAll('gu-island').forEach((element) => { + for (const element of document.querySelectorAll('gu-island')) { if (element instanceof HTMLElement) { const name = getName(element); const props = getProps(element); + const config = getConfig(element); - if (!name) return; - if (element.getAttribute('deferuntil') === 'hash') return; + if (!name) continue; + if (element.getAttribute('deferuntil') === 'hash') continue; import( /* webpackInclude: /\.importable\.tsx$/ */ @@ -34,7 +37,11 @@ export const doStorybookHydration = () => { .querySelector('[data-name="placeholder"]') ?.remove(); const root = createRoot(element); - root.render(createElement(module[name], props)); + root.render( + + {createElement(module[name], props)} + , + ); }) .catch((e) => // eslint-disable-next-line no-console -- We want to log here @@ -44,5 +51,5 @@ export const doStorybookHydration = () => { ), ); } - }); + } }; diff --git a/dotcom-rendering/src/client/islands/getConfig.ts b/dotcom-rendering/src/client/islands/getConfig.ts new file mode 100644 index 00000000000..a6fdef28848 --- /dev/null +++ b/dotcom-rendering/src/client/islands/getConfig.ts @@ -0,0 +1,28 @@ +import type { Config } from '../../types/configContext'; + +/** + * getConfig takes the given html element and returns its config attribute + * + * We expect the element to always be a `gu-*` custom element + * + * @param marker : The html element that we want to read the config attribute from; + * @returns + */ +export const getConfig = (marker: HTMLElement): Config => { + const serialised = marker.getAttribute('config'); + + try { + if (!serialised) { + throw Error('Unable to fetch config attribute from marker element'); + } else { + return JSON.parse(serialised) as Config; + } + } catch (error: unknown) { + console.error( + `🚨 Error parsing config. Is this data serialisable? ${String( + serialised, + )} 🚨`, + ); + throw error; + } +}; diff --git a/dotcom-rendering/src/client/islands/initHydration.ts b/dotcom-rendering/src/client/islands/initHydration.ts index 5b73e2cc2a9..b8b2bdbec25 100644 --- a/dotcom-rendering/src/client/islands/initHydration.ts +++ b/dotcom-rendering/src/client/islands/initHydration.ts @@ -1,5 +1,6 @@ import type { EmotionCache } from '@emotion/cache'; import { doHydration } from './doHydration'; +import { getConfig } from './getConfig'; import { getName } from './getName'; import { getProps } from './getProps'; import { onInteraction } from './onInteraction'; @@ -36,6 +37,7 @@ export const initHydration = async ( ): Promise => { const name = getName(element); const props = getProps(element); + const config = getConfig(element); if (!name) return; @@ -43,7 +45,7 @@ export const initHydration = async ( switch (deferUntil) { case 'idle': { whenIdle(() => { - void doHydration(name, props, element, emotionCache); + void doHydration(name, props, element, emotionCache, config); }); return; } @@ -52,7 +54,13 @@ export const initHydration = async ( whenVisible( element, () => { - void doHydration(name, props, element, emotionCache); + void doHydration( + name, + props, + element, + emotionCache, + config, + ); }, { rootMargin }, ); @@ -60,17 +68,21 @@ export const initHydration = async ( } case 'interaction': { onInteraction(element, (targetElement) => { - void doHydration(name, props, element, emotionCache).then( - () => { - targetElement.dispatchEvent(new MouseEvent('click')); - }, - ); + void doHydration( + name, + props, + element, + emotionCache, + config, + ).then(() => { + targetElement.dispatchEvent(new MouseEvent('click')); + }); }); return; } case 'hash': { if (window.location.hash.includes(name) || hasLightboxHash(name)) { - void doHydration(name, props, element, emotionCache); + void doHydration(name, props, element, emotionCache, config); } else { // If we didn't find a matching hash on page load, set a // listener so that we check again each time the reader @@ -80,14 +92,20 @@ export const initHydration = async ( window.location.hash.includes(name) || hasLightboxHash(name) ) { - void doHydration(name, props, element, emotionCache); + void doHydration( + name, + props, + element, + emotionCache, + config, + ); } }); } return; } default: { - return doHydration(name, props, element, emotionCache); + return doHydration(name, props, element, emotionCache, config); } } }; diff --git a/dotcom-rendering/src/components/ArticleBody.tsx b/dotcom-rendering/src/components/ArticleBody.tsx index c04988b07fc..36ee2760922 100644 --- a/dotcom-rendering/src/components/ArticleBody.tsx +++ b/dotcom-rendering/src/components/ArticleBody.tsx @@ -10,7 +10,6 @@ import { revealStyles } from '../lib/revealStyles'; import type { ServerSideTests, Switches } from '../types/config'; import type { TableOfContentsItem } from '../types/frontend'; import type { Palette } from '../types/palette'; -import type { RenderingTarget } from '../types/renderingTarget'; import type { TagType } from '../types/tag'; import { Island } from './Island'; import { RecipeMultiplier } from './RecipeMultiplier.importable'; @@ -47,7 +46,6 @@ type Props = { tableOfContents?: TableOfContentsItem[]; lang?: string; isRightToLeftLang?: boolean; - renderingTarget: RenderingTarget; }; const globalH2Styles = (display: ArticleDisplay) => css` @@ -139,7 +137,6 @@ export const ArticleBody = ({ tableOfContents, lang, isRightToLeftLang = false, - renderingTarget, }: Props) => { const isInteractive = format.design === ArticleDesign.Interactive; const palette = decidePalette(format); @@ -240,7 +237,6 @@ export const ArticleBody = ({ isAdFreeUser={isAdFreeUser} isSensitive={isSensitive} abTests={abTests} - renderingTarget={renderingTarget} /> diff --git a/dotcom-rendering/src/components/ArticleHeadline.stories.tsx b/dotcom-rendering/src/components/ArticleHeadline.stories.tsx index 0bace46f79b..c1f50204551 100644 --- a/dotcom-rendering/src/components/ArticleHeadline.stories.tsx +++ b/dotcom-rendering/src/components/ArticleHeadline.stories.tsx @@ -38,7 +38,6 @@ export const ArticleStory = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -65,7 +64,6 @@ export const Feature = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -98,7 +96,6 @@ export const ShowcaseInterview = () => { tags={[]} webPublicationDateDeprecated="" byline="Byline text" - renderingTarget="Web" /> { tags={[]} webPublicationDateDeprecated="" byline="" - renderingTarget="Web" /> { tags={[]} webPublicationDateDeprecated="" byline="Byline text" - renderingTarget="Web" /> { tags={[]} webPublicationDateDeprecated="" byline="Byline text" - renderingTarget="Web" /> { tags={[]} webPublicationDateDeprecated="" byline="" - renderingTarget="Web" /> { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -350,7 +342,6 @@ export const Analysis = () => { format={format(theme)} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -381,7 +372,6 @@ export const Gallery = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -408,7 +398,6 @@ export const Review = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -435,7 +424,6 @@ export const PhotoEssay = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -462,7 +450,6 @@ export const Explainer = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -489,7 +476,6 @@ export const Quiz = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -516,7 +502,6 @@ export const Recipe = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -543,7 +528,6 @@ export const Immersive = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -570,7 +554,6 @@ export const ImmersiveNoMainMedia = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -602,7 +585,6 @@ export const ImmersiveComment = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -629,7 +611,6 @@ export const Editorial = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -656,7 +637,6 @@ export const MatchReport = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -683,7 +663,6 @@ export const SpecialReport = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -710,7 +689,6 @@ export const SpecialReportAlt = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -737,7 +715,6 @@ export const LiveBlog = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -777,7 +754,6 @@ export const DeadBlog = () => { format={format} tags={[]} webPublicationDateDeprecated="" - renderingTarget="Web" /> @@ -805,7 +781,6 @@ export const ReviewWithoutStars = () => { tags={[]} webPublicationDateDeprecated="" byline="Byline text" - renderingTarget="Web" /> { }, ]} webPublicationDateDeprecated="2020-03-28T07:27:19.000Z" - renderingTarget="Web" /> diff --git a/dotcom-rendering/src/components/ArticleHeadline.tsx b/dotcom-rendering/src/components/ArticleHeadline.tsx index b619be13847..ddf3e82c2c1 100644 --- a/dotcom-rendering/src/components/ArticleHeadline.tsx +++ b/dotcom-rendering/src/components/ArticleHeadline.tsx @@ -13,7 +13,6 @@ import { getAgeWarning } from '../lib/age-warning'; import { decidePalette } from '../lib/decidePalette'; import { getZIndex } from '../lib/getZIndex'; import type { Palette } from '../types/palette'; -import type { RenderingTarget } from '../types/renderingTarget'; import type { TagType } from '../types/tag'; import { AgeWarning } from './AgeWarning'; import { DesignTag } from './DesignTag'; @@ -28,7 +27,6 @@ type Props = { hasStarRating?: boolean; hasAvatar?: boolean; isMatch?: boolean; - renderingTarget: RenderingTarget; }; const topPadding = css` @@ -342,7 +340,6 @@ export const ArticleHeadline = ({ hasStarRating, hasAvatar, isMatch, - renderingTarget, }: Props) => { const palette = decidePalette(format); switch (format.display) { @@ -418,7 +415,6 @@ export const ArticleHeadline = ({ format={format} byline={byline} tags={tags} - renderingTarget={renderingTarget} /> )} @@ -574,7 +570,6 @@ export const ArticleHeadline = ({ format={format} byline={byline} tags={tags} - renderingTarget={renderingTarget} /> )} @@ -665,7 +660,6 @@ export const ArticleHeadline = ({ format={format} byline={byline} tags={tags} - renderingTarget={renderingTarget} /> )} @@ -706,7 +700,6 @@ export const ArticleHeadline = ({ format={format} byline={byline} tags={tags} - renderingTarget={renderingTarget} /> )} diff --git a/dotcom-rendering/src/components/ArticleMeta.stories.tsx b/dotcom-rendering/src/components/ArticleMeta.stories.tsx index 56dd6f14f36..44f3b0a6002 100644 --- a/dotcom-rendering/src/components/ArticleMeta.stories.tsx +++ b/dotcom-rendering/src/components/ArticleMeta.stories.tsx @@ -85,12 +85,38 @@ export const ArticleStory = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); }; +export const ArticleAppsStory = () => { + return ( + + + + ); +}; +/** @see /dotcom-rendering/docs/development/storybook.md */ +ArticleAppsStory.args = { config: { renderingTarget: 'Apps' } }; + export const BrandingStory = () => { return ( @@ -128,7 +154,6 @@ export const BrandingStory = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); @@ -156,7 +181,6 @@ export const FeatureStory = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); @@ -183,7 +207,6 @@ export const FeatureWithMismatchedContributor = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); @@ -211,7 +234,6 @@ export const FeatureStoryWithSmallBylineImage = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); @@ -238,7 +260,6 @@ export const SpecialReportStory = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); @@ -265,7 +286,6 @@ export const SpecialReportAlt = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); @@ -292,7 +312,6 @@ export const CommentStory = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); @@ -319,7 +338,6 @@ export const InterviewStory = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); @@ -346,7 +364,6 @@ export const ImmersiveStory = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); @@ -373,7 +390,6 @@ export const TwoContributorsStory = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); @@ -402,7 +418,6 @@ export const DeadBlogStory = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ))} @@ -431,7 +446,6 @@ export const Dateline = () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" /> ); diff --git a/dotcom-rendering/src/components/ArticleMeta.test.tsx b/dotcom-rendering/src/components/ArticleMeta.test.tsx index efff27f0944..ddefa7c6874 100644 --- a/dotcom-rendering/src/components/ArticleMeta.test.tsx +++ b/dotcom-rendering/src/components/ArticleMeta.test.tsx @@ -33,7 +33,6 @@ describe('ArticleMeta', () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" />, ); @@ -74,7 +73,6 @@ describe('ArticleMeta', () => { shortUrlId="" ajaxUrl="" showShareCount={true} - renderingTarget="Web" />, ); diff --git a/dotcom-rendering/src/components/ArticleMeta.tsx b/dotcom-rendering/src/components/ArticleMeta.tsx index c44da76f2ff..6ce405a31d6 100644 --- a/dotcom-rendering/src/components/ArticleMeta.tsx +++ b/dotcom-rendering/src/components/ArticleMeta.tsx @@ -14,7 +14,6 @@ import { getSoleContributor } from '../lib/byline'; import { decidePalette } from '../lib/decidePalette'; import type { Branding as BrandingType } from '../types/branding'; import type { Palette } from '../types/palette'; -import type { RenderingTarget } from '../types/renderingTarget'; import type { TagType } from '../types/tag'; import { Avatar } from './Avatar'; import { Branding } from './Branding.importable'; @@ -42,7 +41,6 @@ type Props = { ajaxUrl: string; showShareCount: boolean; messageUs?: MessageUs; - renderingTarget: RenderingTarget; }; const meta = (format: ArticleFormat) => { @@ -315,7 +313,6 @@ export const ArticleMeta = ({ ajaxUrl, showShareCount, messageUs, - renderingTarget, }: Props) => { const soleContributor = getSoleContributor(tags, byline); const authorName = soleContributor?.title ?? 'Author Image'; @@ -380,7 +377,6 @@ export const ArticleMeta = ({ byline={byline} tags={tags} format={format} - renderingTarget={renderingTarget} /> )} {messageUs && diff --git a/dotcom-rendering/src/components/ArticlePage.tsx b/dotcom-rendering/src/components/ArticlePage.tsx index b5538b552e9..fa5007a0fde 100644 --- a/dotcom-rendering/src/components/ArticlePage.tsx +++ b/dotcom-rendering/src/components/ArticlePage.tsx @@ -40,7 +40,7 @@ interface AppProps extends BaseProps { /** * @description * Article is a high level wrapper for article pages on Dotcom. Sets strict mode and some globals - * */ + */ export const ArticlePage = (props: WebProps | AppProps) => { const { article, format, renderingTarget } = props; diff --git a/dotcom-rendering/src/components/BylineLink.test.tsx b/dotcom-rendering/src/components/BylineLink.test.tsx index 3637ec74880..cb930043e9d 100644 --- a/dotcom-rendering/src/components/BylineLink.test.tsx +++ b/dotcom-rendering/src/components/BylineLink.test.tsx @@ -111,7 +111,6 @@ describe('BylineLink', () => { design: ArticleDesign.Standard, theme: Pillar.News, }} - renderingTarget="Web" />, ); @@ -145,7 +144,6 @@ describe('BylineLink', () => { design: ArticleDesign.Standard, theme: Pillar.News, }} - renderingTarget="Web" />, ); @@ -181,7 +179,6 @@ describe('BylineLink', () => { design: ArticleDesign.Standard, theme: Pillar.News, }} - renderingTarget="Web" />, ); diff --git a/dotcom-rendering/src/components/BylineLink.tsx b/dotcom-rendering/src/components/BylineLink.tsx index fd489cba84a..dd074021507 100644 --- a/dotcom-rendering/src/components/BylineLink.tsx +++ b/dotcom-rendering/src/components/BylineLink.tsx @@ -4,8 +4,8 @@ import { getSoleContributor, isContributor, } from '../lib/byline'; -import type { RenderingTarget } from '../types/renderingTarget'; import type { TagType } from '../types/tag'; +import { useConfig } from './ConfigContext'; import { FollowWrapper } from './FollowWrapper.importable'; import { Island } from './Island'; @@ -13,7 +13,6 @@ type Props = { byline: string; tags: TagType[]; format: ArticleFormat; - renderingTarget: RenderingTarget; }; const applyCleverOrderingForMatching = (titles: string[]): string[] => { @@ -107,12 +106,7 @@ function removeComma(bylinePart: string) { : bylinePart; } -export const BylineLink = ({ - byline, - tags, - format, - renderingTarget, -}: Props) => { +export const BylineLink = ({ byline, tags, format }: Props) => { const tokens = bylineAsTokens(byline, tags); const soleContributor = getSoleContributor(tags, byline); const hasSoleContributor = !!soleContributor; @@ -137,6 +131,12 @@ export const BylineLink = ({ ); }); + /** + * Where is this coming from? + * Config value is set at high in the component tree within a React context in a `` + */ + const { renderingTarget } = useConfig(); + return ( <> {renderedTokens} diff --git a/dotcom-rendering/src/components/ConfigContext.test.tsx b/dotcom-rendering/src/components/ConfigContext.test.tsx new file mode 100644 index 00000000000..85b40baca79 --- /dev/null +++ b/dotcom-rendering/src/components/ConfigContext.test.tsx @@ -0,0 +1,45 @@ +import { render } from '@testing-library/react'; + +// This file is globally mocked in "/dotcom-rendering/scripts/jest/setup.ts" +// so we need to explicitly override this in order to test its functionality +const { useConfig, ConfigProvider } = jest.requireActual('./ConfigContext.tsx'); + +const testId = 'testId'; +const TestComponent = () => { + const config = useConfig(); + return ( +
+

{JSON.stringify(config)}

+
+ ); +}; + +describe('ConfigContext', () => { + describe('without ConfigProvider', () => { + it('does not allow use of the useConfig hook without being inside ConfigProvider', () => { + expect(() => TestComponent()).toThrowError(); + }); + }); + + describe('with ConfigProvider', () => { + it.each(['Web', 'Apps'])( + 'provides correct context through useConfig hook with renderingTarget: "%s"', + (renderingTarget) => { + const config = { renderingTarget }; + const Component = () => { + return ( + + + + ); + }; + const { getByTestId, getByText } = render(); + + expect(getByTestId(testId)).toBeInTheDocument(); + expect(getByText(/renderingTarget/)).toHaveTextContent( + renderingTarget, + ); + }, + ); + }); +}); diff --git a/dotcom-rendering/src/components/ConfigContext.tsx b/dotcom-rendering/src/components/ConfigContext.tsx new file mode 100644 index 00000000000..468e7a898a7 --- /dev/null +++ b/dotcom-rendering/src/components/ConfigContext.tsx @@ -0,0 +1,40 @@ +import { createContext, useContext } from 'react'; +import type { Config } from '../types/configContext'; + +/** + * Context for global, static, immutable values (application configuration) + * This should not contain anything which will change between re-renders. + * + * It is deliberately set with a default context of `undefined` to better + * surface errors relating to incorrect usage. + * + * It is deliberately not exported + * @see https://kentcdodds.com/blog/how-to-use-react-context-effectively + */ +const ConfigContext = createContext(undefined); + +/** + * ConfigProvider component should be included at a high level + * so that lower down components can access the context value + */ +export const ConfigProvider = ({ + value, + children, +}: { + value: Config; + children: React.ReactNode; +}) => {children}; + +/** + * useContext hook for safely fetching application configuration. + * Ensures that it is used within the relevant Context.Provider + */ +export const useConfig = (): Config => { + const context = useContext(ConfigContext); + + if (context === undefined) { + throw Error('useConfig must be used within the ConfigContext provider'); + } + + return context; +}; diff --git a/dotcom-rendering/src/components/ContentABTest.amp.tsx b/dotcom-rendering/src/components/ContentABTest.amp.tsx index 8ab92291537..ee9fcb5bcc7 100644 --- a/dotcom-rendering/src/components/ContentABTest.amp.tsx +++ b/dotcom-rendering/src/components/ContentABTest.amp.tsx @@ -58,6 +58,7 @@ export const ContentABTestProvider = ({ children: React.ReactNode; }) => { const group = getGroup(pageId); + // eslint-disable-next-line react/jsx-no-constructed-context-values -- TODO consider using useMemo? const providerValue = switches.ampContentAbTesting ? { group } : {}; return ( {children} diff --git a/dotcom-rendering/src/components/Contributor.test.tsx b/dotcom-rendering/src/components/Contributor.test.tsx index 4d4fea56b98..31afd436522 100644 --- a/dotcom-rendering/src/components/Contributor.test.tsx +++ b/dotcom-rendering/src/components/Contributor.test.tsx @@ -24,7 +24,6 @@ describe('Contributor', () => { title: 'Observer Design', }, ]} - renderingTarget="Web" />, ); @@ -51,7 +50,6 @@ describe('Contributor', () => { title: 'Observer Design', }, ]} - renderingTarget="Web" />, ); diff --git a/dotcom-rendering/src/components/Contributor.tsx b/dotcom-rendering/src/components/Contributor.tsx index 117bb4358f4..861c5958f77 100644 --- a/dotcom-rendering/src/components/Contributor.tsx +++ b/dotcom-rendering/src/components/Contributor.tsx @@ -6,7 +6,6 @@ import { getSoleContributor } from '../lib/byline'; import { decidePalette } from '../lib/decidePalette'; import TwitterIcon from '../static/icons/twitter.svg'; import type { Palette } from '../types/palette'; -import type { RenderingTarget } from '../types/renderingTarget'; import type { TagType } from '../types/tag'; import { BylineLink } from './BylineLink'; @@ -96,15 +95,9 @@ type Props = { byline: string; tags: TagType[]; format: ArticleFormat; - renderingTarget: RenderingTarget; }; -export const Contributor = ({ - byline, - tags, - format, - renderingTarget, -}: Props) => { +export const Contributor = ({ byline, tags, format }: Props) => { const palette = decidePalette(format); const { twitterHandle } = getSoleContributor(tags, byline) ?? {}; @@ -127,12 +120,7 @@ export const Contributor = ({ bylineColorStyles(palette, format), ]} > - + )} {!!twitterHandle && ( diff --git a/dotcom-rendering/src/components/GetMatchNav.importable.tsx b/dotcom-rendering/src/components/GetMatchNav.importable.tsx index 934e3bff527..095ff645a5f 100644 --- a/dotcom-rendering/src/components/GetMatchNav.importable.tsx +++ b/dotcom-rendering/src/components/GetMatchNav.importable.tsx @@ -3,7 +3,6 @@ import { ArticleDesign } from '@guardian/libs'; import { from } from '@guardian/source-foundations'; import type { SWRConfiguration } from 'swr'; import { useApi } from '../lib/useApi'; -import type { RenderingTarget } from '../types/renderingTarget'; import type { TagType } from '../types/tag'; import { ArticleHeadline } from './ArticleHeadline'; import { cleanTeamData } from './GetMatchStats.importable'; @@ -16,7 +15,6 @@ type Props = { format: ArticleFormat; tags: TagType[]; webPublicationDateDeprecated: string; - renderingTarget: RenderingTarget; }; const Loading = () => ; @@ -39,7 +37,6 @@ export const GetMatchNav = ({ format, tags, webPublicationDateDeprecated, - renderingTarget, }: Props) => { const options: SWRConfiguration = { errorRetryCount: 1 }; // If this blog is live then poll for new stats @@ -82,7 +79,6 @@ export const GetMatchNav = ({ webPublicationDateDeprecated } isMatch={true} - renderingTarget={renderingTarget} /> ); diff --git a/dotcom-rendering/src/components/HeadlineByline.stories.tsx b/dotcom-rendering/src/components/HeadlineByline.stories.tsx index 776e406f385..7edef52b17e 100644 --- a/dotcom-rendering/src/components/HeadlineByline.stories.tsx +++ b/dotcom-rendering/src/components/HeadlineByline.stories.tsx @@ -22,7 +22,6 @@ export const interviewStory = () => { }} byline="Jane Smith" tags={[]} - renderingTarget="Web" /> ); }; @@ -38,7 +37,6 @@ export const commentStory = () => { }} byline="Jane Smith" tags={[]} - renderingTarget="Web" /> ); }; @@ -54,7 +52,6 @@ export const specialStory = () => { }} byline="Jane Smith" tags={[]} - renderingTarget="Web" /> ); }; @@ -80,7 +77,6 @@ export const commentWithBylineImageStory = () => { 'https://i.guim.co.uk/img/uploads/2018/01/10/Marina_Hyde,_L.png?width=300&quality=85&auto=format&fit=max&s=6476202195914952e48ef41aadb116ff', }, ]} - renderingTarget="Web" /> ); }; @@ -102,7 +98,6 @@ export const immersiveStory = () => { title: 'Jane Smith', }, ]} - renderingTarget="Web" /> ); }; @@ -130,7 +125,6 @@ export const ImmersiveComment = () => { title: 'Jane Smith', }, ]} - renderingTarget="Web" /> ); @@ -163,7 +157,6 @@ export const MultipleStory = () => { title: 'Nae Bevan', }, ]} - renderingTarget="Web" /> ); }; @@ -190,7 +183,6 @@ export const MultipleDuplicateStory = () => { title: 'Duncan Campbell', }, ]} - renderingTarget="Web" /> ); }; @@ -207,7 +199,6 @@ export const noBylineStory = () => { }} byline="" tags={[]} - renderingTarget="Web" /> ); }; @@ -229,7 +220,6 @@ export const LabsImmersive = () => { title: 'Jane Smith', }, ]} - renderingTarget="Web" /> ); }; @@ -251,7 +241,6 @@ export const LabsComment = () => { title: 'Jane Smith', }, ]} - renderingTarget="Web" /> ); }; @@ -273,7 +262,6 @@ export const LabsInterview = () => { title: 'Jane Smith', }, ]} - renderingTarget="Web" /> ); }; diff --git a/dotcom-rendering/src/components/HeadlineByline.tsx b/dotcom-rendering/src/components/HeadlineByline.tsx index 0b85d5baf7b..d393eb56a91 100644 --- a/dotcom-rendering/src/components/HeadlineByline.tsx +++ b/dotcom-rendering/src/components/HeadlineByline.tsx @@ -12,7 +12,6 @@ import { import { getSoleContributor } from '../lib/byline'; import { decidePalette } from '../lib/decidePalette'; import type { Palette } from '../types/palette'; -import type { RenderingTarget } from '../types/renderingTarget'; import type { TagType } from '../types/tag'; import { BylineLink } from './BylineLink'; @@ -145,15 +144,9 @@ type Props = { format: ArticleFormat; byline: string; tags: TagType[]; - renderingTarget: RenderingTarget; }; -export const HeadlineByline = ({ - format, - byline, - tags, - renderingTarget, -}: Props) => { +export const HeadlineByline = ({ format, byline, tags }: Props) => { if (byline === '') { return null; } @@ -172,7 +165,6 @@ export const HeadlineByline = ({ byline={byline} tags={tags} format={format} - renderingTarget={renderingTarget} /> @@ -190,7 +182,6 @@ export const HeadlineByline = ({ byline={byline} tags={tags} format={format} - renderingTarget={renderingTarget} /> @@ -210,7 +201,6 @@ export const HeadlineByline = ({ byline={byline} tags={tags} format={format} - renderingTarget={renderingTarget} /> @@ -228,7 +218,6 @@ export const HeadlineByline = ({ byline={byline} tags={tags} format={format} - renderingTarget={renderingTarget} /> diff --git a/dotcom-rendering/src/components/Island.tsx b/dotcom-rendering/src/components/Island.tsx index df8fc7ad0b4..e80684d0aae 100644 --- a/dotcom-rendering/src/components/Island.tsx +++ b/dotcom-rendering/src/components/Island.tsx @@ -1,3 +1,4 @@ +import { useConfig } from './ConfigContext'; import { Placeholder } from './Placeholder'; /** @@ -133,18 +134,29 @@ export const Island = ({ placeholderHeight, rootMargin, children, -}: Props) => ( - - {decideChildren(children, clientOnly, placeholderHeight)} - -); +}: Props) => { + /** + * Where is this coming from? + * Config value is set at high in the component tree within a React context in a `` + * + * This is here so that we can provide the config information to the hydrated, client-side rendered components + */ + const config = useConfig(); + + return ( + + {decideChildren(children, clientOnly, placeholderHeight)} + + ); +}; /** * If JavaScript is disabled, hide client-only islands diff --git a/dotcom-rendering/src/layouts/CommentLayout.tsx b/dotcom-rendering/src/layouts/CommentLayout.tsx index 711ca9e71c9..0399dd4fc74 100644 --- a/dotcom-rendering/src/layouts/CommentLayout.tsx +++ b/dotcom-rendering/src/layouts/CommentLayout.tsx @@ -47,7 +47,6 @@ import { decideTrail } from '../lib/decideTrail'; import { parse } from '../lib/slot-machine-flags'; import type { NavType } from '../model/extract-nav'; import type { FEArticleType } from '../types/frontend'; -import type { RenderingTarget } from '../types/renderingTarget'; import { BannerWrapper, SendToBack, Stuck } from './lib/stickiness'; const StandardGrid = ({ @@ -262,15 +261,9 @@ interface Props { article: FEArticleType; NAV: NavType; format: ArticleFormat; - renderingTarget: RenderingTarget; } -export const CommentLayout = ({ - article, - NAV, - format, - renderingTarget, -}: Props) => { +export const CommentLayout = ({ article, NAV, format }: Props) => { const { config: { isPaidContent, host }, } = article; @@ -490,7 +483,6 @@ export const CommentLayout = ({ 'number' } hasAvatar={!!avatarUrl} - renderingTarget={renderingTarget} /> {/* BOTTOM */}
@@ -559,7 +551,6 @@ export const CommentLayout = ({ !!article.config.switches .serverShareCounts } - renderingTarget={renderingTarget} />
@@ -600,7 +591,6 @@ export const CommentLayout = ({ isRightToLeftLang={ article.isRightToLeftLang } - renderingTarget={renderingTarget} /> {showBodyEndSlot && ( diff --git a/dotcom-rendering/src/layouts/DecideLayout.tsx b/dotcom-rendering/src/layouts/DecideLayout.tsx index 45bd1f64ead..5279dfcdf07 100644 --- a/dotcom-rendering/src/layouts/DecideLayout.tsx +++ b/dotcom-rendering/src/layouts/DecideLayout.tsx @@ -23,17 +23,11 @@ interface AppProps extends BaseProps { } interface WebProps extends BaseProps { - renderingTarget: 'Web'; NAV: NavType; + renderingTarget: 'Web'; } -const DecideLayoutApps = ({ - article, - format, -}: { - article: FEArticleType; - format: ArticleFormat; -}) => { +const DecideLayoutApps = ({ article, format, renderingTarget }: AppProps) => { const notSupported =
Not supported
; switch (format.display) { case ArticleDisplay.Standard: { @@ -43,7 +37,7 @@ const DecideLayoutApps = ({ ); case ArticleDesign.LiveBlog: @@ -52,7 +46,7 @@ const DecideLayoutApps = ({ ); default: @@ -92,7 +86,6 @@ const DecideLayoutWeb = ({ article={article} NAV={NAV} format={format} - renderingTarget={renderingTarget} /> ); } @@ -119,7 +112,6 @@ const DecideLayoutWeb = ({ article={article} NAV={NAV} format={format} - renderingTarget={renderingTarget} /> ); default: @@ -128,7 +120,6 @@ const DecideLayoutWeb = ({ article={article} NAV={NAV} format={format} - renderingTarget={renderingTarget} /> ); } @@ -142,7 +133,6 @@ const DecideLayoutWeb = ({ article={article} NAV={NAV} format={format} - renderingTarget={renderingTarget} /> ); case ArticleDesign.FullPageInteractive: { @@ -161,7 +151,7 @@ const DecideLayoutWeb = ({ article={article} NAV={NAV} format={format} - renderingTarget="Web" + renderingTarget={renderingTarget} /> ); case ArticleDesign.Comment: @@ -172,7 +162,6 @@ const DecideLayoutWeb = ({ article={article} NAV={NAV} format={format} - renderingTarget={renderingTarget} /> ); case ArticleDesign.NewsletterSignup: @@ -181,7 +170,6 @@ const DecideLayoutWeb = ({ article={article} NAV={NAV} format={format} - renderingTarget={renderingTarget} /> ); default: @@ -198,12 +186,18 @@ const DecideLayoutWeb = ({ } }; -export const DecideLayout = (props: AppProps | WebProps) => { +export const DecideLayout = (props: WebProps | AppProps) => { const { article, format, renderingTarget } = props; switch (renderingTarget) { case 'Apps': - return ; + return ( + + ); case 'Web': return ( ( @@ -184,7 +183,6 @@ interface Props { article: FEArticleType; NAV: NavType; format: ArticleFormat; - renderingTarget: RenderingTarget; } const decideCaption = (mainMedia: FEElement | undefined): string => { @@ -243,12 +241,7 @@ const Box = ({ ); -export const ImmersiveLayout = ({ - article, - NAV, - format, - renderingTarget, -}: Props) => { +export const ImmersiveLayout = ({ article, NAV, format }: Props) => { const { config: { isPaidContent, host }, } = article; @@ -460,7 +453,6 @@ export const ImmersiveLayout = ({ hasStarRating={ article.starRating !== undefined } - renderingTarget={renderingTarget} /> @@ -539,7 +531,6 @@ export const ImmersiveLayout = ({ typeof article.starRating === 'number' } - renderingTarget={renderingTarget} /> )} @@ -557,7 +548,6 @@ export const ImmersiveLayout = ({ format={format} tags={article.tags} byline={article.byline} - renderingTarget={renderingTarget} /> )} @@ -609,7 +599,6 @@ export const ImmersiveLayout = ({ !!article.config.switches .serverShareCounts } - renderingTarget={renderingTarget} /> @@ -647,7 +636,6 @@ export const ImmersiveLayout = ({ isRightToLeftLang={ article.isRightToLeftLang } - renderingTarget={renderingTarget} /> {showBodyEndSlot && ( diff --git a/dotcom-rendering/src/layouts/InteractiveLayout.tsx b/dotcom-rendering/src/layouts/InteractiveLayout.tsx index 187659b6a22..29f4e95d920 100644 --- a/dotcom-rendering/src/layouts/InteractiveLayout.tsx +++ b/dotcom-rendering/src/layouts/InteractiveLayout.tsx @@ -48,7 +48,6 @@ import { decidePalette } from '../lib/decidePalette'; import { decideTrail } from '../lib/decideTrail'; import type { NavType } from '../model/extract-nav'; import type { FEArticleType } from '../types/frontend'; -import type { RenderingTarget } from '../types/renderingTarget'; import { interactiveGlobalStyles, interactiveLegacyClasses, @@ -204,15 +203,9 @@ interface Props { article: FEArticleType; NAV: NavType; format: ArticleFormat; - renderingTarget: RenderingTarget; } -export const InteractiveLayout = ({ - article, - NAV, - format, - renderingTarget, -}: Props) => { +export const InteractiveLayout = ({ article, NAV, format }: Props) => { const { config: { isPaidContent, host }, } = article; @@ -439,7 +432,6 @@ export const InteractiveLayout = ({ typeof article.starRating === 'number' } - renderingTarget={renderingTarget} /> {article.starRating !== undefined ? ( @@ -492,7 +484,6 @@ export const InteractiveLayout = ({ !!article.config.switches .serverShareCounts } - renderingTarget={renderingTarget} /> @@ -531,7 +522,6 @@ export const InteractiveLayout = ({ isRightToLeftLang={ article.isRightToLeftLang } - renderingTarget={renderingTarget} /> diff --git a/dotcom-rendering/src/layouts/LiveLayout.tsx b/dotcom-rendering/src/layouts/LiveLayout.tsx index 6fda765a71a..7147c54c8ed 100644 --- a/dotcom-rendering/src/layouts/LiveLayout.tsx +++ b/dotcom-rendering/src/layouts/LiveLayout.tsx @@ -61,6 +61,7 @@ import { decideTrail } from '../lib/decideTrail'; import { getZIndex } from '../lib/getZIndex'; import type { NavType } from '../model/extract-nav'; import type { FEArticleType } from '../types/frontend'; +import type { RenderingTarget } from '../types/renderingTarget'; import { BannerWrapper, SendToBack, Stuck } from './lib/stickiness'; const HeadlineGrid = ({ children }: { children: React.ReactNode }) => ( @@ -244,6 +245,7 @@ const paddingBody = css` interface BaseProps { article: FEArticleType; format: ArticleFormat; + renderingTarget: RenderingTarget; } interface AppsProps extends BaseProps { @@ -251,8 +253,8 @@ interface AppsProps extends BaseProps { } interface WebProps extends BaseProps { - renderingTarget: 'Web'; NAV: NavType; + renderingTarget: 'Web'; } export const LiveLayout = (props: WebProps | AppsProps) => { @@ -461,7 +463,6 @@ export const LiveLayout = (props: WebProps | AppsProps) => { webPublicationDateDeprecated={ article.webPublicationDateDeprecated } - renderingTarget={renderingTarget} /> @@ -498,7 +499,6 @@ export const LiveLayout = (props: WebProps | AppsProps) => { typeof article.starRating === 'number' } - renderingTarget={renderingTarget} /> )} @@ -585,7 +585,6 @@ export const LiveLayout = (props: WebProps | AppsProps) => { .serverShareCounts } messageUs={article.messageUs} - renderingTarget={renderingTarget} /> @@ -750,7 +749,6 @@ export const LiveLayout = (props: WebProps | AppsProps) => { .serverShareCounts } messageUs={article.messageUs} - renderingTarget={renderingTarget} /> @@ -904,9 +902,6 @@ export const LiveLayout = (props: WebProps | AppsProps) => { .serverSideLiveblogInlineAdsVariant === 'variant' } - renderingTarget={ - renderingTarget - } /> {pagination.totalPages > 1 && ( { isRightToLeftLang={ article.isRightToLeftLang } - renderingTarget={ - renderingTarget - } /> {pagination.totalPages > 1 && ( : undefined, ); -export const NewsletterSignupLayout = ({ - article, - NAV, - format, - renderingTarget, -}: Props) => { +export const NewsletterSignupLayout = ({ article, NAV, format }: Props) => { const { promotedNewsletter, config: { host }, @@ -403,7 +396,6 @@ export const NewsletterSignupLayout = ({ webPublicationDateDeprecated={ article.webPublicationDateDeprecated } - renderingTarget={renderingTarget} /> { +export const ShowcaseLayout = ({ article, NAV, format }: Props) => { const { config: { isPaidContent, host }, } = article; @@ -578,7 +571,6 @@ export const ShowcaseLayout = ({ hasStarRating={ article.starRating !== undefined } - renderingTarget={renderingTarget} /> @@ -623,7 +615,6 @@ export const ShowcaseLayout = ({ !!article.config.switches .serverShareCounts } - renderingTarget={renderingTarget} /> @@ -661,7 +652,6 @@ export const ShowcaseLayout = ({ isRightToLeftLang={ article.isRightToLeftLang } - renderingTarget={renderingTarget} /> {showBodyEndSlot && ( diff --git a/dotcom-rendering/src/layouts/StandardLayout.tsx b/dotcom-rendering/src/layouts/StandardLayout.tsx index 64f8dd5d078..d0e530af279 100644 --- a/dotcom-rendering/src/layouts/StandardLayout.tsx +++ b/dotcom-rendering/src/layouts/StandardLayout.tsx @@ -340,7 +340,7 @@ export const StandardLayout = (props: WebProps | AppProps) => { return ( <> - {renderingTarget === 'Web' && ( + {isWeb && (
{renderAds && ( @@ -502,7 +502,6 @@ export const StandardLayout = (props: WebProps | AppProps) => { webPublicationDateDeprecated={ article.webPublicationDateDeprecated } - renderingTarget={renderingTarget} /> )} @@ -569,7 +568,6 @@ export const StandardLayout = (props: WebProps | AppProps) => { hasStarRating={ typeof article.starRating === 'number' } - renderingTarget={renderingTarget} />
@@ -628,7 +626,6 @@ export const StandardLayout = (props: WebProps | AppProps) => { !!article.config.switches .serverShareCounts } - renderingTarget={renderingTarget} /> @@ -667,7 +664,6 @@ export const StandardLayout = (props: WebProps | AppProps) => { isRightToLeftLang={ article.isRightToLeftLang } - renderingTarget={renderingTarget} /> {format.design === ArticleDesign.MatchReport && !!footballMatchUrl && ( diff --git a/dotcom-rendering/src/lib/ArticleRenderer.tsx b/dotcom-rendering/src/lib/ArticleRenderer.tsx index fbbd6c16c5b..ce404c87ee6 100644 --- a/dotcom-rendering/src/lib/ArticleRenderer.tsx +++ b/dotcom-rendering/src/lib/ArticleRenderer.tsx @@ -2,10 +2,10 @@ import { css } from '@emotion/react'; import type { ArticleFormat } from '@guardian/libs'; import { ArticleDesign } from '@guardian/libs'; import { adContainerStyles } from '../components/AdSlot'; +import { useConfig } from '../components/ConfigContext'; import { interactiveLegacyClasses } from '../layouts/lib/interactiveLegacyStyling'; import type { ServerSideTests, Switches } from '../types/config'; import type { FEElement } from '../types/content'; -import type { RenderingTarget } from '../types/renderingTarget'; import type { TagType } from '../types/tag'; import { RenderArticleElement } from './renderElement'; import { withSignInGateSlot } from './withSignInGateSlot'; @@ -42,7 +42,6 @@ type Props = { isAdFreeUser: boolean; isSensitive: boolean; abTests?: ServerSideTests; - renderingTarget: RenderingTarget; }; export const ArticleRenderer = ({ @@ -63,7 +62,6 @@ export const ArticleRenderer = ({ isSensitive, isDev, abTests, - renderingTarget, }: Props) => { const renderedElements = elements.map((element, index) => { return ( @@ -86,6 +84,12 @@ export const ArticleRenderer = ({ ); }); + /** + * Where is this coming from? + * Config value is set at high in the component tree within a React context in a `` + */ + const { renderingTarget } = useConfig(); + // const cleanedElements = elements.map(element => // 'html' in element ? { ...element, html: clean(element.html) } : element, // ); diff --git a/dotcom-rendering/src/server/render.allEditorialNewslettersPage.web.tsx b/dotcom-rendering/src/server/render.allEditorialNewslettersPage.web.tsx index f8f87b11f4b..f5485545767 100644 --- a/dotcom-rendering/src/server/render.allEditorialNewslettersPage.web.tsx +++ b/dotcom-rendering/src/server/render.allEditorialNewslettersPage.web.tsx @@ -1,4 +1,5 @@ import { AllEditorialNewslettersPage } from '../components/AllEditorialNewslettersPage'; +import { ConfigProvider } from '../components/ConfigContext'; import { generateScriptTags, getModulesBuild, @@ -9,6 +10,7 @@ import { getHttp3Url } from '../lib/getHttp3Url'; import { polyfillIO } from '../lib/polyfill.io'; import { extractNAV } from '../model/extract-nav'; import { createGuardian } from '../model/guardian'; +import type { Config } from '../types/configContext'; import type { DCRNewslettersPageType } from '../types/newslettersPage'; import { htmlPageTemplate } from './htmlPageTemplate'; @@ -22,11 +24,16 @@ export const renderEditorialNewslettersPage = ({ const title = newslettersPage.webTitle; const NAV = extractNAV(newslettersPage.nav); + // The newsletters page is currently only supported on Web + const config: Config = { renderingTarget: 'Web' }; + const { html, extractedCss } = renderToStringWithEmotion( - , + + + , ); // Evaluating the performance of HTTP3 over HTTP2 diff --git a/dotcom-rendering/src/server/render.article.amp.tsx b/dotcom-rendering/src/server/render.article.amp.tsx index a4139fb21cf..de3f90ef278 100644 --- a/dotcom-rendering/src/server/render.article.amp.tsx +++ b/dotcom-rendering/src/server/render.article.amp.tsx @@ -5,9 +5,11 @@ import { resets } from '@guardian/source-foundations'; import he from 'he'; import React from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; +import { ConfigProvider } from '../components/ConfigContext'; import { epicChoiceCardCss } from '../components/Epic.amp'; import { stickyAdLabelCss } from '../components/StickyAd.amp'; import { getFontsCss } from '../lib/fonts-css'; +import type { Config } from '../types/configContext'; interface RenderToStringResult { html: string; @@ -36,9 +38,15 @@ export const renderArticle = ({ const cache = createCache({ key }); // eslint-disable-next-line @typescript-eslint/unbound-method const { extractCritical } = createEmotionServer(cache); + + // We are currently considering AMP to be a renderingTarget of Web + const config: Config = { renderingTarget: 'Web' }; + const { html, css }: RenderToStringResult = extractCritical( renderToStaticMarkup( - {body}, + + {body} + , ), ); diff --git a/dotcom-rendering/src/server/render.article.apps.tsx b/dotcom-rendering/src/server/render.article.apps.tsx index c031c6cf2d7..28bab1f545e 100644 --- a/dotcom-rendering/src/server/render.article.apps.tsx +++ b/dotcom-rendering/src/server/render.article.apps.tsx @@ -1,9 +1,11 @@ import { isString } from '@guardian/libs'; import { ArticlePage } from '../components/ArticlePage'; +import { ConfigProvider } from '../components/ConfigContext'; import { generateScriptTags, getPathFromManifest } from '../lib/assets'; import { decideFormat } from '../lib/decideFormat'; import { renderToStringWithEmotion } from '../lib/emotion'; import { createGuardian } from '../model/guardian'; +import type { Config } from '../types/configContext'; import type { FEArticleType } from '../types/frontend'; import { htmlPageTemplate } from './htmlPageTemplate'; @@ -15,12 +17,17 @@ export const renderArticle = ( } => { const format: ArticleFormat = decideFormat(article.format); + const renderingTarget = 'Apps'; + const config: Config = { renderingTarget }; + const { html, extractedCss } = renderToStringWithEmotion( - , + + + , ); const clientScripts = [getPathFromManifest('apps', 'index.js')]; diff --git a/dotcom-rendering/src/server/render.article.web.tsx b/dotcom-rendering/src/server/render.article.web.tsx index ddf722b2f2c..b1767d5ad4e 100644 --- a/dotcom-rendering/src/server/render.article.web.tsx +++ b/dotcom-rendering/src/server/render.article.web.tsx @@ -1,5 +1,6 @@ import { ArticleDesign, isString, Pillar } from '@guardian/libs'; import { ArticlePage } from '../components/ArticlePage'; +import { ConfigProvider } from '../components/ConfigContext'; import { isAmpSupported } from '../components/Elements.amp'; import { KeyEventsContainer } from '../components/KeyEventsContainer'; import { @@ -18,6 +19,7 @@ import { polyfillIO } from '../lib/polyfill.io'; import { extractGA } from '../model/extract-ga'; import { extractNAV } from '../model/extract-nav'; import { createGuardian as createWindowGuardian } from '../model/guardian'; +import type { Config } from '../types/configContext'; import type { FEElement } from '../types/content'; import type { FEArticleType, FEBlocksRequest } from '../types/frontend'; import type { TagType } from '../types/tag'; @@ -47,13 +49,18 @@ export const renderHtml = ({ const format: ArticleFormat = decideFormat(article.format); + const renderingTarget = 'Web'; + const config: Config = { renderingTarget }; + const { html, extractedCss } = renderToStringWithEmotion( - , + + + , ); // We want to only insert script tags for the elements or main media elements on this page view @@ -248,26 +255,31 @@ export const renderBlocks = ({ }: FEBlocksRequest): string => { const format: ArticleFormat = decideFormat(FEFormat); + // Only currently supported for Web + const config: Config = { renderingTarget: 'Web' }; + const { html, extractedCss } = renderToStringWithEmotion( - , + + + , ); return `${extractedCss}${html}`; @@ -284,12 +296,16 @@ export const renderKeyEvents = ({ format: FEFormat, filterKeyEvents, }: FEKeyEventsRequest): string => { + const config: Config = { renderingTarget: 'Web' }; + const { html, extractedCss } = renderToStringWithEmotion( - , + + + , ); return `${extractedCss}${html}`; diff --git a/dotcom-rendering/src/server/render.front.web.tsx b/dotcom-rendering/src/server/render.front.web.tsx index 98fe5442622..157491a6d59 100644 --- a/dotcom-rendering/src/server/render.front.web.tsx +++ b/dotcom-rendering/src/server/render.front.web.tsx @@ -1,4 +1,5 @@ import { isString, Pillar } from '@guardian/libs'; +import { ConfigProvider } from '../components/ConfigContext'; import { FrontPage } from '../components/FrontPage'; import { TagFrontPage } from '../components/TagFrontPage'; import { @@ -13,6 +14,7 @@ import { themeToPillar } from '../lib/themeToPillar'; import type { NavType } from '../model/extract-nav'; import { extractNAV } from '../model/extract-nav'; import { createGuardian } from '../model/guardian'; +import type { Config } from '../types/configContext'; import type { DCRFrontType } from '../types/front'; import type { DCRTagFrontType } from '../types/tagFront'; import { htmlPageTemplate } from './htmlPageTemplate'; @@ -75,8 +77,13 @@ export const renderFront = ({ const title = front.webTitle; const NAV = extractFrontNav(front); + // Fronts are not supported in Apps + const config: Config = { renderingTarget: 'Web' }; + const { html, extractedCss } = renderToStringWithEmotion( - , + + , + , ); // Evaluating the performance of HTTP3 over HTTP2 @@ -164,8 +171,13 @@ export const renderTagFront = ({ const title = tagFront.webTitle; const NAV = extractNAV(tagFront.nav); + // Fronts are not supported in Apps + const config: Config = { renderingTarget: 'Web' }; + const { html, extractedCss } = renderToStringWithEmotion( - , + + , + , ); // Evaluating the performance of HTTP3 over HTTP2 diff --git a/dotcom-rendering/src/types/configContext.ts b/dotcom-rendering/src/types/configContext.ts new file mode 100644 index 00000000000..98579c0a438 --- /dev/null +++ b/dotcom-rendering/src/types/configContext.ts @@ -0,0 +1,11 @@ +import type { RenderingTarget } from './renderingTarget'; + +/** + * Context for global, static, immutable values i.e. application configuration + * + * This should not contain any properties which are likely to change between re-renders + * @see /dotcom-rendering/docs/architecture/proposed-adrs/react-context-api.md + */ +export interface Config { + renderingTarget: RenderingTarget; +} diff --git a/dotcom-rendering/stories/generated/Layout.stories.tsx b/dotcom-rendering/stories/generated/Layout.stories.tsx index 89016c39757..d2c3d22f621 100644 --- a/dotcom-rendering/stories/generated/Layout.stories.tsx +++ b/dotcom-rendering/stories/generated/Layout.stories.tsx @@ -28,6 +28,7 @@ export const WebStandardStandardNewsPillar = () => { ); }; WebStandardStandardNewsPillar.storyName = 'Web: Display: Standard, Design: Standard, Theme: NewsPillar'; +WebStandardStandardNewsPillar.args = { config: { renderingTarget: 'Web' } }; export const AppsStandardStandardNewsPillar = () => { return ( @@ -40,6 +41,7 @@ export const AppsStandardStandardNewsPillar = () => { ); }; AppsStandardStandardNewsPillar.storyName = 'Apps: Display: Standard, Design: Standard, Theme: NewsPillar'; +AppsStandardStandardNewsPillar.args = { config: { renderingTarget: 'Apps' } }; export const WebShowcasePictureOpinionPillar = () => { return ( @@ -52,3 +54,4 @@ export const WebShowcasePictureOpinionPillar = () => { ); }; WebShowcasePictureOpinionPillar.storyName = 'Web: Display: Showcase, Design: Picture, Theme: OpinionPillar'; +WebShowcasePictureOpinionPillar.args = { config: { renderingTarget: 'Web' } };