gadicc / meteor-blaze-react-component

<Blaze template="itemsList" items={items} />
https://atmospherejs.com/gadicc/blaze-react-component
MIT License
61 stars 12 forks source link

Package won't work with React 17 #21

Closed vjau closed 3 years ago

vjau commented 4 years ago

React 17 will deprecate componentWillReceiveProps which this package uses. In the actual version of React, you get a warning. In react 17 componentWillReceiveProps should be prefixed with UNSAFE_ to work. I don't know enough of reactlifecycle/blaze/ this package to help with the rewrite to avoid componentWillReceiveProps.

gadicc commented 4 years ago

Hey @vjau, nice to see you here and thanks for the early heads up.

From a quick look, it should be enough to move the contents of componentWillReceiveProps() to the start of shouldComponentUpdate(). It will be a little while until I can look at this properly, but at least we have a good idea of what to do.

Funnily enough, componentWillReceiveProps, with all it's "unsafe" patterns in traditional React use, actually was a perfect fit for this use case, with no side effects. But it should work just as well in shouldComponentUpdate, even if less semantically clear.

What's news from Meteor land? What kind of projects are you still using this with?

vjau commented 4 years ago

Hi @gadicc, i don't know if you have heard, but MDG has sold Meteor to Tiny. That means the development will be able to go on. Your package is still the recommended way by the Meteor guide to use the login package in React apps, since it has not been ported to React.

gadicc commented 4 years ago

Yes, I heard about the acquisition, hope it will go in a good direction. And thanks, didn't realise this was still the best way to use the account packages.

I made the change and all tests are still passing the test suite (actually, hardest part was upgrading Meteor and all related packages and test packages :)). I published as gadicc:blaze-react-component@1.4.3-pre.0, I'd be grateful if you could try this in a bigger project and see if everything still works as expected!

vjau commented 4 years ago

Hi @gadicc , sorry for the late answer, and thank you for your reactivity. I have just tested your fix with my limited use case which is the login gui module, but for this use, it seems to work. Thank you very much !

edemaine commented 3 years ago

FWIW, I've tried it in a few places in my app, and it works well. Perhaps worth finally releasing as a non-beta?

gadicc commented 3 years ago

Right you are, @edemaine, thanks for the nudge :) And thanks again @vjau (belatedly) for your help with this issue! Released as 1.4.3.