search

BerkeleyTrue / react-redux-epic

37 stars 13 forks source link

version 0.1.0 to 0.3.0 #6

Closed cometta closed 7 years ago

cometta commented 7 years ago

I get below error, any example for using renderToString().subscribe(..) for 0.3.0? It was working on 0.1.0

/build/node_modules/rxjs/Subscriber.js:240
            throw err;
            ^

TypeError: Cannot set property 'length' of null
    at Subject.complete (/build/node_modules/rxjs/Subject.js:85:31)
BerkeleyTrue commented 7 years ago

@cometta Is there a stack trace that comes along with that error? It looks like it is only pointing to Rx code.

BerkeleyTrue commented 7 years ago

@cometta I'm able to replicate the error. Let me look into a fix

BerkeleyTrue commented 7 years ago

This one was a hard one to track down.

Looks like the behavior of Subject.prototype.complete was changed between Rx beta used in 0.2.0 and Rx v5 used in 0.3.0 so that it no longer creates a local var of the observers before eventually calling unsubscribe.

This causes a serious of events within renderToString which causes the error above:

  1. wrappedRootEpic.complete is called in initialRender.
  2. This calls actionsProxy.complete which intern causes the actions obs' passed into the user epic to complete, which itself triggers the lifecycle.complete. This all happens synchronously.
  3. lifecycle is a subject so it's complete will cycle through all the observers synchronously one by one and call their complete method here.
  4. This synchronously triggers the last and map operators in renderToString.
  5. The map operator will call wrappedRootEpic.unsubscribe.
  6. wrappedRootEpic will then call lifecycle.unsubscribe which will null the observers property. Keep in mind that this is all happening synchronously which means that somewhere in the call stack the lifecycle.complete function has yet to resolve and is waiting for all of it's observers to complete. This is the last function on the call stack to finish. Once it resolves we start working our way back down the stack.
  7. Once lifecycle observers have completed it will then set the length of the observers array to zero (this effectively blanks our the array). But since the lifecycle.unsubscribe function was called before this function could complete it will attempt to set the length property of a null object causing the error.

tl:dr; We need to make sure the wrappedRootEpic.unsubscribe happens after the wrappedRootEpic.complete function has resolved.

I've got a fix for this but I want to get a failing test before I submit it.

cometta commented 7 years ago

@BerkeleyTrue ,if you create a branch of the changes, i can help you to test it.

BerkeleyTrue commented 7 years ago

@cometta I've got a PR going through travis if you want to check it out

BerkeleyTrue commented 7 years ago

Fix released as 0.3.1

cometta commented 7 years ago

yup, it's working in 0.3.1, thank you @BerkeleyTrue

BerkeleyTrue commented 7 years ago

👍