Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DNM] React 19 #12374

Merged
merged 42 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
35b449b
Testing
amoore108 Apr 26, 2024
9648799
Merge branch 'latest' into react-19
amoore108 Jan 21, 2025
86469a4
cache
amoore108 Jan 21, 2025
96d2f1f
Merge branch 'latest' into react-19
amoore108 Jan 21, 2025
aa37e2f
Merge branch 'latest' into react-19
amoore108 Jan 31, 2025
e8ec6b9
cache
amoore108 Jan 31, 2025
b3ce3e3
Merge branch 'latest' into react-19
amoore108 Feb 5, 2025
614b98d
Update `@types/react`
amoore108 Feb 5, 2025
ec8b0ee
Update dependabot.yml
amoore108 Feb 5, 2025
49e7035
Update Emotion to support React 19 types
amoore108 Feb 5, 2025
97b493c
run yarn dedupe
amoore108 Feb 5, 2025
2b74064
Type fixes
amoore108 Feb 5, 2025
2a993e0
viewRef updates
amoore108 Feb 5, 2025
bd852c8
more viewRef fixes
amoore108 Feb 5, 2025
b8b9f54
more viewRef updates
amoore108 Feb 5, 2025
1a17d46
Try `React.JSX` namespace for type declaration files
amoore108 Feb 5, 2025
5d5f437
Update bundleSizeConfig.js
amoore108 Feb 5, 2025
40351ab
Update index.jsx
amoore108 Feb 5, 2025
729657c
Merge branch 'latest' into react-19
amoore108 Feb 6, 2025
e7f67cc
cache
amoore108 Feb 6, 2025
dd44c2e
Merge branch 'latest' into react-19
amoore108 Feb 6, 2025
ad087dc
cache
amoore108 Feb 6, 2025
d794555
Syntax and test fixes
amoore108 Feb 6, 2025
c92e2f7
Update index.tsx
amoore108 Feb 6, 2025
8a5649a
Update component.test.jsx.snap
amoore108 Feb 6, 2025
ce8c8d8
Disable AMP boilerplate test temporarily
amoore108 Feb 6, 2025
a70b5b5
Merge branch 'latest' into react-19
amoore108 Feb 6, 2025
626822d
Update client.js
amoore108 Feb 6, 2025
ac78180
Update client.js
amoore108 Feb 6, 2025
b89620a
Merge branch 'latest' into react-19
amoore108 Feb 6, 2025
eb78848
Try retries for Chartbeat timeouts
amoore108 Feb 6, 2025
3a12d05
Revert "Try retries for Chartbeat timeouts"
amoore108 Feb 6, 2025
28e1395
Force reload of window for Chartbeat script tests
amoore108 Feb 7, 2025
9815a25
Merge branch 'latest' into react-19
amoore108 Feb 7, 2025
0cf3f72
Test with retries on Chartbeat e2e
amoore108 Feb 7, 2025
18ab422
Remove eslint ignores for `imagesrcset` and `imageSizes`
amoore108 Feb 7, 2025
f755b9c
AMP boilerplate test refactor
amoore108 Feb 7, 2025
d09fd1d
Missed file
amoore108 Feb 7, 2025
6d5f90f
Remove `fetchpriority` from eslint ignore
amoore108 Feb 10, 2025
287291a
Merge branch 'latest' into react-19
amoore108 Feb 10, 2025
ec9b915
Update bundleSizeConfig.js
amoore108 Feb 10, 2025
2064d8d
Merge branch 'latest' into react-19
amoore108 Feb 10, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,6 @@ updates:
- dependency-name: 'react-router'
# https://jira.dev.bbc.co.uk/browse/NEWSWORLDSERVICE-2099: Delete path-to-regexp dependency
- dependency-name: 'path-to-regexp'
# https://jira.dev.bbc.co.uk/browse/NEWSWORLDSERVICE-2187: Latest major version of react has breaking changes
- dependency-name: 'react'
update-types: ['version-update:semver-major']
- dependency-name: 'react-dom'
update-types: ['version-update:semver-major']
- dependency-name: '@types/react'
update-types: ['version-update:semver-major']
- dependency-name: '@types/react-dom'
update-types: ['version-update:semver-major']
karinathomasbbc marked this conversation as resolved.
Show resolved Hide resolved
labels:
- 'dependencies'
groups:
Expand Down
2 changes: 1 addition & 1 deletion .storybook/DocsDecorator/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { type JSX } from 'react';
import {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to import React here for storybook if we're removing it from imports in components below?

i.e in the Next app at ws-nextjs-app/pages/[service]/send/[id]/MessageBox/ErrorSummaryBox.tsx

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea we'll need to here. Other components that don't import will likely be importing something like:

/** @jsx jsx */
import { jsx } from '@emotion/react';

which means Emotion will be taking control of JSX rendering.

DocsContainer,
DocsContextProps,
Expand Down
2 changes: 1 addition & 1 deletion .storybook/withServicesDecorator/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { type JSX } from 'react';
import { Helmet } from 'react-helmet';
pvaliani marked this conversation as resolved.
Show resolved Hide resolved
import { StoryProps } from '#app/models/types/storybook';
import { Services, Variants } from '#app/models/types/global';
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file added .yarn/cache/fsevents-patch-6b67494872-10.zip
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
11 changes: 6 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
"@optimizely/js-sdk-utils": "0.4.0",
"uuid": "3.4.0",
"[email protected]": "patch:winston@npm:3.8.2#.yarn/patches/winston-npm-3.8.2-2035e9cac4.patch",
"cookie": "0.7.1"
"cookie": "0.7.1",
"react-is": "19.0.0"
},
"workspaces": {
"packages": [
Expand Down Expand Up @@ -120,8 +121,8 @@
"morgan": "1.10.0",
"polyfill-crypto.getrandomvalues": "1.0.0",
"ramda": "0.30.1",
"react": "18.3.1",
"react-dom": "18.3.1",
"react": "19.0.0",
"react-dom": "19.0.0",
"react-helmet": "6.1.0",
"react-lazyload": "3.2.1",
"react-router-config": "5.1.1",
Expand Down Expand Up @@ -185,8 +186,8 @@
"@types/jsdom": "21.1.7",
"@types/loadable__component": "5.13.9",
"@types/ramda": "0.30.2",
"@types/react": "18.3.3",
"@types/react-dom": "18.3.1",
"@types/react": "19.0.8",
"@types/react-dom": "19.0.3",
"@types/react-helmet": "6.1.11",
"@types/react-lazyload": "^3.2.0",
"@types/react-router-dom": "5.3.3",
Expand Down
4 changes: 2 additions & 2 deletions scripts/bundleSize/bundleSizeConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@
* We are allowing a variance of -5 on `MIN_SIZE` and +5 on `MAX_SIZE` to avoid the need for frequent changes, as bundle sizes can fluctuate
*/

export const MIN_SIZE = 640 - 5;
export const MAX_SIZE = 1174 + 5;
export const MIN_SIZE = 681 - 5;
export const MAX_SIZE = 1216 + 5;
2 changes: 1 addition & 1 deletion src/app/components/ATIAnalytics/amp/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
declare namespace JSX {
declare namespace React.JSX {
/*
* AMP currently doesn't have built-in types for TypeScript, but it's in their roadmap (https://github.com/ampproject/amphtml/issues/13791).
* As a workaround you can manually create custom types (https://stackoverflow.com/a/50601125).
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/Ad/Amp/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
declare namespace JSX {
declare namespace React.JSX {
/*
* AMP currently doesn't have built-in types for TypeScript, but it's in their roadmap (https://github.com/ampproject/amphtml/issues/13791).
* As a workaround you can manually create custom types (https://stackoverflow.com/a/50601125).
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/AmpExperiment/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
declare namespace JSX {
declare namespace React.JSX {
interface IntrinsicElements {
'amp-experiment': React.PropsWithChildren<
ScriptHTMLAttributes<HTMLScriptElement>
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/AmpIframe/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const AmpIframe = ({
Show more
</button>
</div>
<amp-img layout="fill" src={image} placeholder />
<amp-img layout="fill" src={image} placeholder="true" />
</AmpIframeElement>
</GridItemMedium>
</>
Expand Down
4 changes: 2 additions & 2 deletions src/app/components/Billboard/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/** @jsx jsx */
/* @jsxFrag React.Fragment */
import { forwardRef } from 'react';
import { ForwardedRef, forwardRef } from 'react';
import { jsx } from '@emotion/react';
import useViewTracker from '#app/hooks/useViewTracker';
import useClickTrackerHandler from '#app/hooks/useClickTrackerHandler';
Expand Down Expand Up @@ -35,7 +35,7 @@ const Billboard = forwardRef(
eventTrackingData,
showLiveLabel,
}: BillboardProps,
viewRef,
viewRef: ForwardedRef<HTMLDivElement>,
) => {
const clickTrackerHandler = useClickTrackerHandler(eventTrackingData);

Expand Down
2 changes: 1 addition & 1 deletion src/app/components/ChartbeatAnalytics/amp/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
declare namespace JSX {
declare namespace React.JSX {
/*
* AMP currently doesn't have built-in types for TypeScript, but it's in their roadmap (https://github.com/ampproject/amphtml/issues/13791).
* As a workaround you can manually create custom types (https://stackoverflow.com/a/50601125).
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/ChartbeatAnalytics/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ describe('Charbeats Analytics Container', () => {
{
chartbeatConfig: expectedConfig,
},
{},
undefined,
);
expect(testUtils.getConfig).toHaveBeenCalledTimes(1);
expect(container.firstChild).not.toBeNull();
Expand Down
21 changes: 14 additions & 7 deletions src/app/components/Curation/CurationPromo/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,20 @@ const CurationPromo = ({

return (
<Promo>
<Promo.Image src={imageUrl} alt={imageAlt} lazyLoad={lazy} isAmp={isAmp}>
{isMedia && (
<Promo.MediaIcon type={type}>
{showDuration ? mediaDuration : ''}
</Promo.MediaIcon>
)}
</Promo.Image>
{imageUrl && (
<Promo.Image
src={imageUrl}
alt={imageAlt}
lazyLoad={lazy}
isAmp={isAmp}
>
{isMedia && (
<Promo.MediaIcon type={type}>
{showDuration ? mediaDuration : ''}
</Promo.MediaIcon>
)}
</Promo.Image>
)}
<Promo.Heading as={`h${headingLevel}`}>
{isMedia ? (
<Promo.A href={link} aria-labelledby={id}>
Expand Down
20 changes: 13 additions & 7 deletions src/app/components/Curation/HierarchicalGrid/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('Hierarchical Grid Curation', () => {

it('should render the last published date', async () => {
const { getByText } = render(
<HierarchicalGrid summaries={mediaFixture} />,
<HierarchicalGrid headingLevel={headingLevel} summaries={mediaFixture} />,
karinathomasbbc marked this conversation as resolved.
Show resolved Hide resolved
{
service: 'mundo',
},
Expand All @@ -109,16 +109,22 @@ describe('Hierarchical Grid Curation', () => {
});

it('should display LiveLabel on a Live Promo', () => {
const container = render(<HierarchicalGrid summaries={mediaFixture} />, {
service: 'mundo',
});
const container = render(
<HierarchicalGrid headingLevel={headingLevel} summaries={mediaFixture} />,
{
service: 'mundo',
},
);
expect(container.getByText('EN VIVO')).toBeInTheDocument();
});

it('should not display a timestamp on a Live Promo', () => {
const container = render(<HierarchicalGrid summaries={liveFixtures} />, {
service: 'mundo',
});
const container = render(
<HierarchicalGrid headingLevel={headingLevel} summaries={liveFixtures} />,
{
service: 'mundo',
},
);
expect(container.queryByText('13 noviembre 2022')).not.toBeInTheDocument();
});
});
12 changes: 7 additions & 5 deletions src/app/components/Curation/HierarchicalGrid/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ const HiearchicalGrid = ({
<Promo>
<Promo.Image
useLargeImages={useLargeImages}
src={promo.imageUrl || ''}
src={promo.imageUrl || null}
alt={promo.imageAlt}
lazyLoad={lazyLoadImages}
fetchpriority={fetchpriority}
fetchPriority={fetchpriority}
isAmp={isAmp}
>
{isMedia && (
Expand Down Expand Up @@ -124,9 +124,11 @@ const HiearchicalGrid = ({
<Promo.A href={promo.link}>
{isLive ? (
<LiveLabel
{...(isFirstPromo && {
className: 'first-promo',
})}
{...(isFirstPromo
? {
className: 'first-promo',
}
: undefined)}
>
{promo.title}
</LiveLabel>
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/EmbedConsentBanner/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState, PropsWithChildren } from 'react';
import React, { useState, PropsWithChildren, type JSX } from 'react';
import ConsentBanner from './ConsentBanner';

import { SocialEmbedProviders } from '../../models/types/global';
Expand Down
1 change: 1 addition & 0 deletions src/app/components/FrostedGlassPromo/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { JSX } from 'react';
import { EventTrackingBlock } from '../../models/types/eventTracking';

export type ImageProps = {
Expand Down
12 changes: 6 additions & 6 deletions src/app/components/Image/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type Props = {
sizes?: string;
src: string;
width?: number;
fetchpriority?: 'high';
fetchPriority?: 'high';
hasCaption?: boolean;
};

Expand Down Expand Up @@ -59,7 +59,7 @@ const Image = ({
src,
width,
children,
fetchpriority,
fetchPriority,
hasCaption,
}: PropsWithChildren<Props>) => {
const { pageType, isLite } = useContext(RequestContext);
Expand Down Expand Up @@ -109,8 +109,8 @@ const Image = ({
rel="preload"
as="image"
href={src}
imagesrcset={srcSet}
imagesizes={sizes}
imageSrcSet={srcSet}
imageSizes={sizes}
amoore108 marked this conversation as resolved.
Show resolved Hide resolved
/>
</Helmet>
)}
Expand Down Expand Up @@ -159,7 +159,7 @@ const Image = ({
attribution={attribution}
{...(srcSet && { srcSet: imgSrcSet })}
{...(imgSizes && { sizes: imgSizes })}
{...(preload && { 'data-hero': true })}
{...(preload && { 'data-hero': 'true' })}
/>
</>
) : (
Expand Down Expand Up @@ -190,7 +190,7 @@ const Image = ({
? styles.imageFixedAspectRatio
: styles.imageResponsiveRatio,
]}
fetchpriority={fetchpriority}
fetchPriority={fetchPriority}
style={{
aspectRatio: hasFixedAspectRatio
? `${aspectRatioX} / ${aspectRatioY}`
Expand Down
15 changes: 4 additions & 11 deletions src/app/components/Image/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
declare namespace JSX {
declare namespace React.JSX {
/*
* AMP currently doesn't have built-in types for TypeScript, but it's in their roadmap (https://github.com/ampproject/amphtml/issues/13791).
* As a workaround you can manually create custom types (https://stackoverflow.com/a/50601125).
Expand All @@ -15,21 +15,14 @@ declare namespace JSX {
src?: string;
srcSet?: string;
width?: number;
placeholder?: boolean;
}
/*
* Overrides type for link with missing imagesrcset and imagesizes attributes
*/
interface LinkProps extends React.LinkHTMLAttributes<HTMLLinkElement> {
imagesrcset?: string;
imagesizes?: string;
placeholder?: string | boolean;
}

interface ImageProps extends React.ImgHTMLAttributes<HTMLImageElement> {
fetchpriority?: string;
fetchPriority?: string;
}
interface IntrinsicElements {
'amp-img': AmpImgProps;
link: LinkProps;
img: ImageProps;
}
}
2 changes: 1 addition & 1 deletion src/app/components/MaskedImage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const MaskedImage = ({
sizes="(min-width: 1008px) 660px, 100vw"
width={800}
height={533}
fetchpriority="high"
fetchPriority="high"
preload
placeholder={showPlaceholder}
/>
Expand Down
1 change: 1 addition & 0 deletions src/app/components/MediaLoader/Amp/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const AmpMediaLoader = ({
>
<div
data-e2e="image-placeholder"
// @ts-expect-error - placeholder is an AMP specific attribute
placeholder=""
css={styles.ampIframePlaceholder}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const Guidance = ({
guidanceMessage && styles.guidanceWrapperWithMessage,
]}
data-e2e="media-player__guidance"
{...(className && { className })}
{...(className ? { className } : undefined)}
>
{guidanceMessage && (
<strong
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const PlayButton = ({
type="button"
css={styles.button}
onClick={onClick}
{...(className && { className })}
{...(className ? { className } : undefined)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, basically this (and similar changes) are making sure we are following the correct spread pattern for React 19 because in 19 dev mode crashes when false is evaluated during spread?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see any crashes as such, tests just failed quite spectacularly without these changes. Not to say it wouldn't cause the app to crash though, it very well could do without noticing as we still display the server render if the app crashes.

>
<VisuallyHiddenText>{hiddenText}</VisuallyHiddenText>
<div
Expand Down
4 changes: 2 additions & 2 deletions src/app/components/MessageBanner/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @jsx jsx */
import { useContext, forwardRef } from 'react';
import { useContext, forwardRef, ForwardedRef } from 'react';
import { jsx } from '@emotion/react';
import useViewTracker from '#app/hooks/useViewTracker';
import { EventTrackingMetadata } from '#app/models/types/eventTracking';
Expand Down Expand Up @@ -32,7 +32,7 @@ const Banner = forwardRef(
eventTrackingData,
id = 'message-banner-1',
}: MessageBannerProps,
viewRef,
viewRef: ForwardedRef<HTMLDivElement>,
) => {
const { dir } = useContext(ServiceContext);
const isRtl = dir === 'rtl';
Expand Down
11 changes: 9 additions & 2 deletions src/app/components/MostRead/Amp/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,18 @@ const AmpMostRead = ({ endpoint, size = 'default' }: AmpMostReadProps) => {
width="300"
height="50"
>
<p css={styles.paragraph} fallback="">
<p
css={styles.paragraph}
// @ts-expect-error - fallback is an AMP specific attribute
fallback=""
>
{fallbackText}
</p>

<template type="amp-mustache">
<template
// @ts-expect-error - type is an AMP specific attribute in this context
type="amp-mustache"
>
<MostReadItemWrapper dir={direction} columnLayout="oneColumn">
<MostReadRank
service={service}
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/MostRead/Amp/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
declare namespace JSX {
declare namespace React.JSX {
/*
* AMP currently doesn't have built-in types for TypeScript, but it's in their roadmap (https://github.com/ampproject/amphtml/issues/13791).
* As a workaround you can manually create custom types (https://stackoverflow.com/a/50601125).
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/MostRead/Section/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const MostReadSection = ({
aria-labelledby="Most-Read"
data-e2e="most-read"
data-testid="most-read"
{...(className && { className })}
{...(className ? { className } : undefined)}
>
{children}
</section>
Expand Down
Loading
Loading