meteor / react-packages

Meteor packages for a great React developer experience
http://guide.meteor.com/react.html
Other
571 stars 157 forks source link

Fix data loss with immediate rerender, and add comparator argument #321

Closed CaptainN closed 3 years ago

CaptainN commented 3 years ago

This adds a comparator argument as the 2nd or third argument.

It also fixes an issue that can occur when an immediate rerender occurs. This can happen when a state setter is called during render. In such cases, React immediately runs the rerender without additional lifecycle stages.

This PR contains tests and readme edits for everything.

CaptainN commented 3 years ago

Tests have been added for an important error with immediate rerenders (calling setState before mount), and for skipUpdate. The errors those tests revealed have been fixed.

The only thing left to do is add some documentation around skipUpdate, but this is otherwise ready to ship.

CaptainN commented 3 years ago

There may be another thing to check for - an immediate rerender after deps change, on an already mounted component. That will probably still yield undefined in the deps version of the hook. I need to write a test for that to confirm, and then probably write a mitigation (which will be much harder).

DavidSichau commented 3 years ago

This also fixes our issue.

CaptainN commented 3 years ago

I updated the readme to include info about skipUpdate. All tests are passing. This is ready to ship.

@filipenevola This also has some important bug fixes, but since it contains a big new feature, it should probably get a minor version bump.

Changes:

CaptainN commented 3 years ago

@menelike It took me a while, but I finally added that comparator option.

afrokick commented 3 years ago

@filipenevola any chance to review&release it on this week?