Remove defaultProps and propTypes

[amoore108]

Overall changes

• Removes defaultProps and propTypes from the codebase as these will become removed/deprecated in React 19+ https://react.dev/blog/2024/04/25/react-19-upgrade-guide#removed-proptypes-and-defaultprops
• Makes use of the ES6 default value to try and replicate the same behaviour as defaultProps before (see the code example in the React docs linked above for a demonstration of this)
• Updates some tests, namely hooks, to use our custom React Testing Library component as some of the functionality in testing-library/react-hooks is becoming deprecated
• Adds a basic Article type to replace the inferProps version previously
• Removes react-test-renderer as it will become deprecated and is not used in the codebase

Code changes

• Removes defaultProps and propTypes declarations from the codebase in favour of using ES6 default values
• This does remove the prop-type type checking, which is kinda similar to TypeScript. This means that older JSX components may not have their props typed anymore. Newer TSX components shouldn't be using prop-types, so they should remain unaffected. There is inference though, so most components will display their props if you CMD+Space or hover over the component.
• Some snapshots have been updated as Emotion added in a blank space before the first classname in some instances. This was resolved by conditionally applying the className prop on affected components

Testing

1. List the steps used to test this PR.

Helpful Links

Add Links to useful resources related to this PR if applicable.

Coding Standards

Repository use guidelines

[amoore108]

Not quite sure how to use ES6 syntax for styled-components like:

simorgh/src/app/legacy/psammead/psammead-bulleted-list/src/index.jsx

Lines 7 to 39 in 12c0e60

	const BulletedList = styled.ul`
	${({ script }) => script && getBodyCopy(script)};
	${({ service }) => getSansRegular(service)};
	margin-top: 0;
	list-style-type: none;
	& > li {
	position: relative;
	color: ${({ theme }) => theme.isDarkUi && theme.palette.GREY_2};
	}
	& > li::before {
	top: 0.5rem;
	content: ' ';
	position: absolute;
	border-width: 1rem;
	border: 0.1875rem solid
	${({ bulletPointColour, theme }) =>
	theme.isDarkUi ? theme.palette.GREY_4 : bulletPointColour};
	background-color: ${({ bulletPointColour, theme }) =>
	theme.isDarkUi ? theme.palette.GREY_4 : bulletPointColour};
	border-radius: ${({ bulletPointShape }) =>
	bulletPointShape === 'round' ? '50%' : '0'};
	${({ dir }) => (dir === 'rtl' ? 'right: -1rem;' : 'left: -1rem;')}
	}
	`;
	BulletedList.defaultProps = {
	dir: 'ltr',
	role: 'list',
	bulletPointShape: 'round',
	bulletPointColour: SHADOW,
	};

May not be a problem if React 19 doesn't shout about it.

[karinathomasbbc]

One question / concern - if we were to ever convert any of these components to Typescrpt, we don't have PropTypes to tell us if fields are required or not.

We will also need to update the Migrate Legacy Component doc to remove any mention of propTypes (if we haven't already done so)

[karinathomasbbc]

Does it matter that this is a required value (it did not have a default value before)?

[amoore108]

Which value are you referencing? linkId had a default of null and serviceLocalisedName had a default of null

[pvaliani]

Copypasta from Slack:

Nice! Thank you.

For the styled components issue with defaultProps, since React isn’t raising flags about their usage currently, it might be safe to leave them as is in the short term. However, for long-term maintenance and consistency, it might be worthwhile to schedule a gradual rewrite of these components into a supported pattern, possibly for when we tackle other refactoring tasks or feature upgrades.

Would it be worthwhile to add additional tests focusing on targeted components where defaultProps were replaced with default parameters to ensure there are no edge cases we're missing where the behaviour could diff?

[andrewscfc]

Huge respect doing this presumably without a codemod! I'm not sure if all my points are necessary, just wanted to point out semantic differences so they can be checked

[amoore108]

One question / concern - if we were to ever convert any of these components to Typescrpt, we don't have PropTypes to tell us if fields are required or not.

We will also need to update the Migrate Legacy Component doc to remove any mention of propTypes (if we haven't already done so)

Thats correct, we won't have a reference to the types that they can accept anymore. This is disadvantage of doing it this way since we have a hybrid of JS and TS. I guess refactoring components to TS will tighten up an loose types over time, but I can't think of any other ways other than something like JSDoc, which would be a huge undertaking.

I will remove the Readme references.

[amoore108]

Copypasta from Slack:

Nice! Thank you.

For the styled components issue with defaultProps, since React isn’t raising flags about their usage currently, it might be safe to leave them as is in the short term. However, for long-term maintenance and consistency, it might be worthwhile to schedule a gradual rewrite of these components into a supported pattern, possibly for when we tackle other refactoring tasks or feature upgrades.

Would it be worthwhile to add additional tests focusing on targeted components where defaultProps were replaced with default parameters to ensure there are no edge cases we're missing where the behaviour could diff?

Thanks, that makes sense. I think we're safe to keep the defaultProps on those components. They are more scarce than other components, so not like we'll have them littered everywhere.

Not really sure if its worth adding more tests. The current ones, along with snapshots/Chromatic should highlight regressions already, so I don't think adding more will help much? Happy to be wrong though!

[andrewscfc]

Thanks for the clarifications and changes, all looks good