Make undux React 18 compatible #160
Conversation
The circleci/node images have been deprecated and replaced by cimg/node according to https://circleci.com/docs/circleci-images/ Node 14 seems to be the most recent version compatible with the webpack version used here.
kassens
changed the title
| ;(this as any).storeDefinition = null | ||
| if (this.subscription != null) { | ||
| this.subscription.unsubscribe() | ||
| this.subscription = null |
There was a problem hiding this comment.
Can we keep the GC lines? These were pretty important for long-running apps with complex state, IIRC.
| this.subscription = null | |
| this.subscription = null | |
| // Let the state get GC'd. | |
| // TODO: Find a more elegant way to do this. | |
| ;(this.storeDefinition as any).storeSnapshot = null | |
| ;(this as any).storeDefinition = null |
There was a problem hiding this comment.
componentDidMount above requires access to the storeDefinition to re-establish the subscription. Not sure there's a way around keeping the reference here?
| ) | ||
| ) as any |
There was a problem hiding this comment.
I can double check, I feel like I had basically just moved these lines but could see if I can fix at least some of them.
| mapValues(this.state.storeSnapshots, _ => ((_ as any).state = null)) | ||
| mapValues( | ||
| this.state.storeSnapshots, | ||
| _ => ((_ as any).storeDefinition = null) |
There was a problem hiding this comment.
Same here -- is there a way we could keep this GC-related code?
There was a problem hiding this comment.
same as above, we need to re-establish the subscriptions.
|
I've just updated all the dependencies to latest. Mind rebasing? Then tests should pass. |
Subscriptions that are torn down in
componentDidUnmountneed to be re-established incomponentDidMountas both functions can be called multiple times in concurrent mode.I was not sure how critical the GC related comments are. I don't know if that behavior can be kept when we need to re-establish the subscriptions.