feat(ESLint): enforce conditional rules

The following rules are now reported as errors: - nullish coalescing - optional chaining This aligns us closer to the Guardian’s house rules
guardian · Jan 19, 2023 · 605a5f6 · 605a5f6
1 parent 018a9f4
commit 605a5f6
Show file tree

Hide file tree

Showing 82 changed files with 207 additions and 236 deletions.
diff --git a/dotcom-rendering/.eslintrc.js b/dotcom-rendering/.eslintrc.js
@@ -23,13 +23,8 @@ const rulesToOverrideGuardianConfig = {
 		},
 	],
 
+	// This is not safe to remove whilst we have noUncheckedIndexedAccess
 	'@typescript-eslint/no-unnecessary-condition': 'warn',
-
-	// use `foo ?? 'a string'` instead of `foo !== null && foo !== undefined ? foo : 'a string'`
-	'@typescript-eslint/prefer-nullish-coalescing': 'warn',
-
-	// use `a?.b` instead of `a && a.b`
-	'@typescript-eslint/prefer-optional-chain': 'warn',
 };
 
 /** @TODO Review these */

diff --git a/dotcom-rendering/scripts/jest/setup.ts b/dotcom-rendering/scripts/jest/setup.ts
@@ -76,7 +76,7 @@ const localStorageMock = (function () {
 	} = {};
 	return {
 		getItem(key: string) {
-			return store[key] || null;
+			return store[key] ?? null;
 		},
 		setItem(key: string, value: string) {
 			store[key] = value.toString();

diff --git a/dotcom-rendering/src/amp/components/Blocks.tsx b/dotcom-rendering/src/amp/components/Blocks.tsx
@@ -152,7 +152,7 @@ export const Blocks = ({
 							>
 								<RegionalAd
 									editionId={editionId}
-									section={section || ''}
+									section={section ?? ''}
 									contentType={contentType}
 									config={adConfig}
 									commercialProperties={commercialProperties}

diff --git a/dotcom-rendering/src/amp/components/BodyArticle.tsx b/dotcom-rendering/src/amp/components/BodyArticle.tsx
@@ -160,7 +160,7 @@ export const Body = ({ data, config }: Props) => {
 							>
 								<RegionalAd
 									editionId={data.editionId}
-									section={data.sectionName || ''}
+									section={data.sectionName ?? ''}
 									contentType={adInfo.contentType}
 									config={adConfig}
 									commercialProperties={
@@ -213,7 +213,7 @@ export const Body = ({ data, config }: Props) => {
 
 			<StickyAd
 				editionId={data.editionId}
-				section={data.sectionName || ''}
+				section={data.sectionName ?? ''}
 				contentType={adInfo.contentType}
 				config={adConfig}
 				commercialProperties={adInfo.commercialProperties}

diff --git a/dotcom-rendering/src/amp/components/Caption.tsx b/dotcom-rendering/src/amp/components/Caption.tsx
@@ -1,5 +1,5 @@
 import { css } from '@emotion/react';
-import { text, textSans } from '@guardian/source-foundations';
+import { neutral, textSans } from '@guardian/source-foundations';
 import React from 'react';
 import { pillarPalette_DO_NOT_USE } from '../../lib/pillars';
 import TriangleIcon from '../../static/icons/triangle.svg';
@@ -11,7 +11,7 @@ const captionStyle = css`
 	padding-top: 10px;
 	${textSans.xxsmall()};
 	word-wrap: break-word;
-	color: ${text.supporting};
+	color: ${neutral[46]};
 `;
 
 const captionPadding = css`
@@ -57,7 +57,7 @@ export const Caption = ({
 			<span
 				css={captionLink}
 				dangerouslySetInnerHTML={{
-					__html: captionText || '',
+					__html: captionText ?? '',
 				}}
 				key="caption"
 			/>

diff --git a/dotcom-rendering/src/amp/components/KeyEvents.tsx b/dotcom-rendering/src/amp/components/KeyEvents.tsx
@@ -78,7 +78,7 @@ export const KeyEvents = ({ events, url }: Props) => {
 		<li css={listItemStyle} key={event.id}>
 			<a css={eventLinkStyle} href={blockLink(url, event.id)}>
 				<span css={timeStyle}>{event.blockCreatedOnDisplay}</span>
-				<span css={listTitleStyle}>{event.title || ''}</span>
+				<span css={listTitleStyle}>{event.title ?? ''}</span>
 			</a>
 		</li>
 	));

diff --git a/dotcom-rendering/src/amp/components/elements/ImageBlockComponent.tsx b/dotcom-rendering/src/amp/components/elements/ImageBlockComponent.tsx
@@ -69,7 +69,7 @@ export const ImageBlockComponent = ({ element, pillar }: Props) => {
 					<span
 						css={captionLink}
 						dangerouslySetInnerHTML={{
-							__html: element.data.caption || '',
+							__html: element.data.caption ?? '',
 						}}
 						key="caption"
 					/>{' '}

diff --git a/dotcom-rendering/src/amp/components/elements/TimelineBlockComponent.tsx b/dotcom-rendering/src/amp/components/elements/TimelineBlockComponent.tsx
@@ -52,7 +52,7 @@ export const TimelineBlockComponent = ({
 	pillar,
 }: Props) => (
 	<Expandable id={id} type="Timeline" title={title} pillar={pillar}>
-		{description || ''}
+		{description ?? ''}
 		<ul css={eventsWrapper}>
 			{events.map((e) => (
 				<li css={eventStyle} key={e.title}>
@@ -67,7 +67,7 @@ export const TimelineBlockComponent = ({
 						<h3 css={headingStyle}>{e.title}</h3>
 						<div
 							dangerouslySetInnerHTML={{
-								__html: e.body || '',
+								__html: e.body ?? '',
 							}}
 						/>
 					</div>

diff --git a/dotcom-rendering/src/amp/lib/get-video-id.ts b/dotcom-rendering/src/amp/lib/get-video-id.ts
@@ -33,7 +33,7 @@ export const getIdFromUrl = (
 
 	if (!id) {
 		return logErr(
-			id || ids.join(', '),
+			id ?? ids.join(', '),
 			`Value(s) didn't match regexFormat ${regexFormat}`,
 		);
 	}

diff --git a/dotcom-rendering/src/amp/lib/image-fit.ts b/dotcom-rendering/src/amp/lib/image-fit.ts
@@ -18,7 +18,7 @@ export const bestFitImage = (
 		.sort((a, b) => a.width - b.width)
 		.find((img) => img.width >= containerWidth);
 
-	return bestFit || srcSets[srcSets.length - 1];
+	return bestFit ?? srcSets[srcSets.length - 1];
 };
 
 // AMP images need their height set explicitly, but

diff --git a/dotcom-rendering/src/lib/ad-targeting.ts b/dotcom-rendering/src/lib/ad-targeting.ts
@@ -35,7 +35,7 @@ export const buildAdTargeting = ({
 	const customParams = {
 		sens: isSensitive ? ('t' as const) : ('f' as const),
 		si: 'f',
-		vl: videoDuration || 0,
+		vl: videoDuration ?? 0,
 		cc: edition,
 		s: section,
 		inskin: 'f',

diff --git a/dotcom-rendering/src/lib/articleCount.ts b/dotcom-rendering/src/lib/articleCount.ts
@@ -45,8 +45,8 @@ export const getArticleCounts = async (
 	}
 
 	return {
-		weeklyArticleHistory: window.guardian.weeklyArticleCount || [],
-		dailyArticleHistory: window.guardian.dailyArticleCount || [],
+		weeklyArticleHistory: window.guardian.weeklyArticleCount ?? [],
+		dailyArticleHistory: window.guardian.dailyArticleCount ?? [],
 	};
 };
 

diff --git a/dotcom-rendering/src/model/enhance-dividers.ts b/dotcom-rendering/src/model/enhance-dividers.ts
@@ -11,7 +11,7 @@ const isDinkus = (element: CAPIElement): boolean => {
 		return false;
 
 	const frag = JSDOM.fragment(element.html);
-	if (!frag || !frag.firstChild) return false;
+	if (!frag.firstChild) return false;
 	// A dinkus is can be spaced or unspaced
 	return (
 		frag.textContent === '***' ||

diff --git a/dotcom-rendering/src/model/enhance-numbered-lists.ts b/dotcom-rendering/src/model/enhance-numbered-lists.ts
@@ -14,7 +14,7 @@ const isFalseH3 = (element: CAPIElement): boolean => {
 		return false;
 	}
 	const frag = JSDOM.fragment(element.html);
-	if (!frag || !frag.firstElementChild) return false;
+	if (!frag.firstElementChild) return false;
 	const html = frag.firstElementChild.outerHTML;
 	// The following things must be true for an element to be a faux H3
 	const hasPwrapper = frag.firstElementChild.nodeName === 'P';
@@ -45,7 +45,7 @@ const extractH3 = (element: CAPIElement): string => {
 				.split('<strong>')
 				.join('')
 				.split('</strong>')
-				.join('') || ''
+				.join('') ?? ''
 		);
 	}
 	return '';
@@ -62,7 +62,7 @@ const isStarRating = (element: CAPIElement): boolean => {
 		return false;
 	const frag = JSDOM.fragment(element.html);
 	const hasPTags = frag.firstElementChild?.nodeName === 'P';
-	const text = frag.textContent || '';
+	const text = frag.textContent ?? '';
 	// Loop the string making sure each letter is a star
 	for (const letter of text) {
 		if (!isStar(letter)) return false;
@@ -78,7 +78,7 @@ const extractStarCount = (element: CAPIElement): number => {
 	// Returns the count of stars
 	const textElement = element as TextBlockElement;
 	const frag = JSDOM.fragment(textElement.html);
-	const text = frag.textContent || '';
+	const text = frag.textContent ?? '';
 	// Loop the string counting selected stars
 	let starCount = 0;
 	for (const letter of text) {
@@ -190,7 +190,7 @@ const isItemLink = (element: CAPIElement): boolean => {
 		return false;
 	}
 	const frag = JSDOM.fragment(element.html);
-	if (!frag || !frag.firstElementChild) return false;
+	if (!frag.firstElementChild) return false;
 
 	const hasULWrapper = frag.firstElementChild.nodeName === 'UL';
 	const hasOnlyOneChild = frag.firstElementChild.childElementCount === 1;

diff --git a/dotcom-rendering/src/model/enhanceCards.ts b/dotcom-rendering/src/model/enhanceCards.ts
@@ -86,8 +86,8 @@ const enhanceSupportingContent = (
 
 		return {
 			format: presentationFormat,
-			headline: subLink.header?.headline || '',
-			url: subLink.properties.href || subLink.header?.url,
+			headline: subLink.header?.headline ?? '',
+			url: subLink.properties.href ?? subLink.header?.url,
 			kickerText: supportingContentIsLive
 				? 'Live'
 				: subLink.header?.kicker?.item?.properties.kickerText,
@@ -168,7 +168,7 @@ export const enhanceCards = (
 	collections.map((faciaCard, index) => {
 		// Snap cards may not have a format, default to a standard format if that's the case.
 		const format = decideFormat(
-			faciaCard.format || {
+			faciaCard.format ?? {
 				design: 'ArticleDesign',
 				theme: 'NewsPillar',
 				display: 'StandardDisplay',

diff --git a/dotcom-rendering/src/model/extract-ga.ts b/dotcom-rendering/src/model/extract-ga.ts
@@ -2,7 +2,7 @@
 
 import type { FEArticleType } from '../types/frontend';
 import type { TagType } from '../types/tag';
-import { EditionId } from '../web/lib/edition';
+import type { EditionId } from '../web/lib/edition';
 
 const filterTags = (
 	tags: FEArticleType['tags'],
@@ -16,6 +16,7 @@ const filterTags = (
 			[],
 		);
 
+	// eslint-disable-next-line @typescript-eslint/prefer-optional-chain -- this code is cursed
 	return (arrOfvalues && arrOfvalues.join(',')) || '';
 };
 
@@ -26,7 +27,7 @@ const getCommissioningDesk = (
 	const tag = tags.find((thisTag) =>
 		thisTag.id.includes('tracking/commissioningdesk'),
 	);
-	return (tag && tag.title) || '';
+	return tag?.title ?? '';
 };
 
 const convertToLegacyPillar = (theme: CAPITheme): LegacyPillar => {
@@ -70,7 +71,7 @@ export const extractGA = ({
 }): GADataType => ({
 	webTitle,
 	pillar: convertToLegacyPillar(format.theme),
-	section: sectionName || '',
+	section: sectionName ?? '',
 	contentType: formatStringForGa(contentType),
 	commissioningDesks: formatStringForGa(getCommissioningDesk(tags)),
 	contentId: pageId,

diff --git a/dotcom-rendering/src/model/find-pillar.ts b/dotcom-rendering/src/model/find-pillar.ts
@@ -6,10 +6,12 @@ export const findPillar: (
 	tags?: TagType[],
 ) => ArticleTheme | undefined = (name, tags?) => {
 	// Flag paid content for Labs pillar (for styling purposes)
-	const isPaidContent = (tag: TagType) =>
-		tag.type === 'Tone' && tag.id === 'tone/advertisement-features';
+	const hasPaidContent = !!tags?.some(
+		({ type, id }) =>
+			type === 'Tone' && id === 'tone/advertisement-features',
+	);
 
-	if (tags && tags.some(isPaidContent)) {
+	if (hasPaidContent) {
 		return ArticleSpecial.Labs;
 	}
 

diff --git a/dotcom-rendering/src/model/insertPromotedNewsletter.ts b/dotcom-rendering/src/model/insertPromotedNewsletter.ts
@@ -161,7 +161,7 @@ const findInsertPosition = (elements: CAPIElement[]): number | null => {
 		.sort(sortPlaces);
 
 	// Return the position to insert the embed or null if none was found
-	return suitablePlacesInOrderOfPrefence[0]?.position || null;
+	return suitablePlacesInOrderOfPrefence[0]?.position ?? null;
 };
 
 /**

diff --git a/dotcom-rendering/src/model/unwrapHtml.ts b/dotcom-rendering/src/model/unwrapHtml.ts
@@ -45,7 +45,7 @@ export const unwrapHtml = ({
 		: html;
 
 	/** `p` is the default unwrappedElement */
-	const unwrappedElement = matchingFix?.unwrappedElement || 'p';
+	const unwrappedElement = matchingFix?.unwrappedElement ?? 'p';
 
 	return {
 		unwrappedHtml,

diff --git a/dotcom-rendering/src/server/prod-server.ts b/dotcom-rendering/src/server/prod-server.ts
@@ -133,7 +133,7 @@ export const prodServer = (): void => {
 		}, 10 * 1000);
 	}
 
-	const port = process.env.PORT || 9000;
+	const port = process.env.PORT ?? 9000;
 	app.listen(port);
 
 	console.log(`Started production server on port ${port}\nready`);

diff --git a/dotcom-rendering/src/web/browser/bootCmp/init.ts b/dotcom-rendering/src/web/browser/bootCmp/init.ts
@@ -22,7 +22,8 @@ const trackPerformance = (
 		return;
 	}
 
-	if (window.performance && window.performance.now) {
+	// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- not supported in all browsers
+	if (window.performance?.now) {
 		ga(
 			'allEditorialPropertyTracker.send',
 			'timing',
@@ -79,7 +80,7 @@ const init = async (): Promise<void> => {
 
 		const decideConsentCarrierLabels = () => {
 			if (consentState.tcfv2) {
-				const consentUUID = getCookie({ name: 'consentUUID' }) || '';
+				const consentUUID = getCookie({ name: 'consentUUID' }) ?? '';
 				const consentString = consentState.tcfv2.tcString;
 				return [
 					'01:TCF.v2',
@@ -88,14 +89,14 @@ const init = async (): Promise<void> => {
 				];
 			}
 			if (consentState.ccpa) {
-				const ccpaUUID = getCookie({ name: 'ccpaUUID' }) || '';
+				const ccpaUUID = getCookie({ name: 'ccpaUUID' }) ?? '';
 				const flag = consentState.ccpa.doNotSell ? 'true' : 'false';
 				return ['01:CCPA', `04:${ccpaUUID}`, `05:${flag}`];
 			}
 			if (consentState.aus) {
-				const ccpaUUID = getCookie({ name: 'ccpaUUID' }) || '';
+				const ccpaUUID = getCookie({ name: 'ccpaUUID' }) ?? '';
 				const consentStatus =
-					getCookie({ name: 'consentStatus' }) || '';
+					getCookie({ name: 'consentStatus' }) ?? '';
 				const personalisedAdvertising = consentState.aus
 					.personalisedAdvertising
 					? 'true'
@@ -165,7 +166,7 @@ const init = async (): Promise<void> => {
 			browserId: browserId ?? undefined,
 			pageViewId,
 		},
-		country: (await getLocaleCode()) || undefined,
+		country: (await getLocaleCode()) ?? undefined,
 	});
 	log('dotcom', 'CMP initialised');
 

diff --git a/dotcom-rendering/src/web/browser/ga/ga.ts b/dotcom-rendering/src/web/browser/ga/ga.ts
@@ -127,7 +127,7 @@ export const sendPageView = (): void => {
 		const NG_STORAGE_KEY = 'gu.analytics.referrerVars';
 		const referrerVarsData = window.sessionStorage.getItem(NG_STORAGE_KEY);
 		const referrerVars: { [key: string]: any } = JSON.parse(
-			referrerVarsData || '{}',
+			referrerVarsData ?? '{}',
 		);
 		if (referrerVars.value) {
 			const value = referrerVars.value as {

diff --git a/dotcom-rendering/src/web/browser/initDiscussion/init.ts b/dotcom-rendering/src/web/browser/initDiscussion/init.ts
@@ -35,7 +35,7 @@ const init = (): Promise<void> => {
 	 *
 	 */
 	const hashLink = window.location.hash;
-	if (hashLink && hashLink.includes('comment')) forceHydration();
+	if (hashLink.includes('comment')) forceHydration();
 
 	return Promise.resolve();
 };

diff --git a/dotcom-rendering/src/web/browser/newsletterEmbedIframe/init.ts b/dotcom-rendering/src/web/browser/newsletterEmbedIframe/init.ts
@@ -13,11 +13,10 @@ const init = (): Promise<void> => {
 	// Otherwise, earlier resize events might be missed
 	// So we don't have to load this script as a priority on each load
 	allIframes.forEach((iframe) => {
-		if (iframe && iframe.contentWindow)
-			iframe.contentWindow.postMessage(
-				'resize',
-				'https://www.theguardian.com',
-			);
+		iframe.contentWindow?.postMessage(
+			'resize',
+			'https://www.theguardian.com',
+		);
 	});
 
 	window.addEventListener('message', (event) => {

diff --git a/dotcom-rendering/src/web/browser/ophan/ophan.ts b/dotcom-rendering/src/web/browser/ophan/ophan.ts
@@ -62,7 +62,7 @@ export const sendOphanComponentEvent = (
 			componentType,
 			products,
 			campaignCode,
-			id: testMeta.campaignId || testMeta.campaignCode,
+			id: testMeta.campaignId ?? testMeta.campaignCode,
 			labels,
 		},
 		abTest: {

diff --git a/dotcom-rendering/src/web/browser/updateIframeHeight.tsx b/dotcom-rendering/src/web/browser/updateIframeHeight.tsx
@@ -31,7 +31,7 @@ export const updateIframeHeight = (queryString: string): Promise<void> => {
 	});
 
 	iframes.forEach((iframe) => {
-		const src = (iframe.getAttribute('srcdoc') || '')
+		const src = (iframe.getAttribute('srcdoc') ?? '')
 			.replace(/<gu-script>/g, '<script>')
 
 			.replace(/<\/gu-script>/g, '<' + '/script>');