Remove `defaultProps` and `propTypes`
Overall changes
- Removes
defaultPropsandpropTypesfrom 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
defaultPropsbefore (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-hooksis becoming deprecated - Adds a basic
Articletype to replace theinferPropsversion previously - Removes
react-test-rendereras it will become deprecated and is not used in the codebase
Code changes
- Removes
defaultPropsandpropTypesdeclarations 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
classNameprop on affected components
Testing
- List the steps used to test this PR.
Helpful Links
Add Links to useful resources related to this PR if applicable.
Not quite sure how to use ES6 syntax for styled-components like: https://github.com/bbc/simorgh/blob/12c0e603660e11029e9b7e6a17caff17e6b4ab9c/src/app/legacy/psammead/psammead-bulleted-list/src/index.jsx#L7-L39
May not be a problem if React 19 doesn't shout about it.
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.
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!