Skip to content

Conversation

@imjordanxd
Copy link

@imjordanxd imjordanxd commented Dec 19, 2024

Overview

Adds React 19 peer dependency support. Also changed a lot of outdated dependencies (some of the open PRs may now be redundant). No runtime code needed to be changed thankfully I changed componentWillMount to componentDidMount and all test pass. This is a good sign. Renovate bot and a yarn GH workflow for this repo would be great!

[
"@babel/preset-react",
{
"runtime": "automatic"
Copy link
Author

Choose a reason for hiding this comment

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

breaking change

"test": "mocha",
"test:watch": "mocha --watch",
"test:cov": "babel-node ./node_modules/.bin/isparta cover ./node_modules/.bin/_mocha"
"test": "jest --forceExit",
Copy link
Author

Choose a reason for hiding this comment

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

there are open handles now and i didn't have enough time to figure out why. It's the node MessageChannel class

babelrc: false,
presets: [
'@babel/preset-react',
['@babel/preset-react', { runtime: 'automatic' }],
Copy link
Author

Choose a reason for hiding this comment

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

Comment on lines -21 to -23
plugins: [
'@babel/plugin-proposal-class-properties',
],
Copy link
Author

Choose a reason for hiding this comment

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

included in @babel/preset-env

]
],
"plugins": [
"@babel/plugin-proposal-class-properties",
Copy link
Author

Choose a reason for hiding this comment

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

@martinnaj
Copy link

Would be nice to remove the UNSAFE_componentWillMount. Seems someone got this working without it nfl/react-helmet#548

@imjordanxd
Copy link
Author

@gaearon can you look at this?

}

class SideEffect extends PureComponent {
class SideEffect extends React.PureComponent {
Copy link
Author

Choose a reason for hiding this comment

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

perhaps in the next major, this could be a functional component

Comment on lines -184 to -212
const originalWindow = global.window;
const originalDocument = global.document;

before(done => {
jsdom.env('<!doctype html><html><head></head><body></body></html>', (error, window) => {
if (!error) {
global.window = window;
global.document = window.document;
}

done(error);
});
});

after(() => {
global.window = originalWindow;
global.document = originalDocument;
});

it('should be findable by react TestUtils', () => {
class AnyComponent extends React.Component {
render() {
return <SideEffect foo="bar" />
}
}
const wrapper = enzyme.shallow(<AnyComponent />);
const sideEffect = wrapper.find(SideEffect)
expect(sideEffect.props()).to.deep.equal({ foo: 'bar' });
});
Copy link
Author

Choose a reason for hiding this comment

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

all of this seemed redundant now

],
"peerDependencies": {
"react": "^16.3.0 || ^17.0.0 || ^18.0.0"
"react": "^16.14.0 || ^17.0.0 || ^18.0.0 || ^19.0.0"
Copy link
Author

Choose a reason for hiding this comment

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

@imjordanxd
Copy link
Author

@gaearon bump! Could you review this? 🙏

@imjordanxd
Copy link
Author

bump @gaearon

@martinnaj
Copy link

All seems fine to me, would be nice if @gaearon could take a look at this

@HichemTabSSCA
Copy link

Any updates on this ?

@gaearon
Copy link
Owner

gaearon commented Dec 25, 2025

What are you using this for? This project should probably be marked as deprecated.

@HichemTabSSCA
Copy link

What are you using this for? This project should probably be marked as deprecated.

It is used by React helmet : https://github.com/nfl/react-helmet/blob/1b57ddb1524bab195df3eeabafef67768a6d2a15/package.json#L38

For me the only problem is that npm shows a warning about unmatched peer dependencies on this package when we use React 19, that's all. So a simple change here would be good (I guess).
Thanks.

@martinnaj
Copy link

Would be nice to merge, as when using rush.js with react-helmet, we have to do shenanigans just to get dep install to pass (with peers enforced)

@gaearon
Copy link
Owner

gaearon commented Dec 26, 2025

React Helmet is also unmaintained and unsupported, I’d recommend either React 19 built-in <head> support or some newer library.

This particular library relies on a technique that doesn’t work very well in modern React, and y’all should really stop using it (as well as React Helmet).

@martinnaj
Copy link

Well React's newest docs still point to helmet, and it's still pretty widely used, while I agree that it shouldn't be, we maintain a lib which supports React 18 + 19, so this is probably still the best way while 18 is still supported :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants