cancelled flow() to reject with new FlowCancellationError

[vonovak]

This exports new FlowCancellationError and isFlowCancellationError helper because it's a little unusual to have to rely on FLOW_CANCELLED in our code base instead of importing this from mobx.

[danielkcz]

Hm, given it's just a string, it doesn't seem to be that big deal. It's a weird method of detecting cancel anyway. It might be better to have a custom FlowCancelError instance that can be checked instead.

[vonovak]

@FredyC yes, it's mostly a stylistic issue. I'd be in favor of having a custom error for this, if you guys prefer - it's just a little more invasive but shouldn't be breaking (if we keep the same message) afaict.

[danielkcz]

it's just a little more invasive but shouldn't be breaking (if we keep the same message) afaict.

That's exactly my line of thinking :) Perhaps even expose something like isFlowCancelError which checks for the error instance. Can you do it, please?

Also, we will need this in mobx4-master branch and update the changelog (only in master).

[vonovak]

@FredyC ok, so I added FlowCancellationError and also isFlowCancellationError(error). Frankly, I'm not sure if such simple function is worth increasing the api surface fo the library, but I'll leave that up to you guys. I can easily revert it.

Also, we will need this in mobx4-master branch and update the changelog (only in master).

crap, forgot about that. So the same changes (just code, not docs) but for different branch?

[danielkcz]

ok, so I added FlowCancellationError and also isFlowCancellationError(error). Frankly, I'm not sure if such simple function is worth increasing the api surface fo the library, but I'll leave that up to you guys. I can easily revert it.

Thanks, I think it's ok. Not exactly the API surface increase, just a cleaner way of handling an existing feature.

crap, forgot about that. So the same changes (just code, not docs) but for different branch?

I am not sure if Flow is handled differently in MobX4, but chances are it's possible to just cherry pick those commits there and won't need much of the changes.

[mweststrate]

I think this is a neat approach indeed. Let's make it part of the next minor

[danielkcz]

Still waiting for MobX4 version, @vonovak will you be able to prepare that?

[vonovak]

@FredyC I'll open a new PR for mobx4 in a few hours.

[vonovak]

@FredyC please see #2190 - please note I had to change the implementation from this

export class FlowCancellationError extends Error {
    constructor() {
        super("FLOW_CANCELLED")
    }
}

to this

export function FlowCancellationError() {
    this.message = "FLOW_CANCELLED"
}
FlowCancellationError.prototype = Object.create(Error.prototype)

as otherwise the tests were failing. Please let me know if you want to use the second impl for this PR too.

[danielkcz]

Thanks!

[mweststrate]

Published as 5.15.0. Thanks!