Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IDerivationState.POSSIBLY_STALE + observableRequiresReaction = warning #2195

Closed
Bnaya opened this issue Nov 8, 2019 · 6 comments · Fixed by #2196
Closed

IDerivationState.POSSIBLY_STALE + observableRequiresReaction = warning #2195

Bnaya opened this issue Nov 8, 2019 · 6 comments · Fixed by #2196
Labels

Comments

@Bnaya
Copy link
Member

Bnaya commented Nov 8, 2019

I don't have a minimal repro or test case yet

Minimal repro:

    test("don't warn on reads inside a computed, when reaction updates the observed value", function () {
        try {
            // #916
            mobx.configure({ observableRequiresReaction: true })
            const a = mobx.observable.box(0)
            const b = mobx.observable.box(0)
            const c = mobx.computed(() => a.get() + b.get())
            const values = []

            mobx.autorun(() => {
                values.push(c.get())
                b.set(100)
            })

            const messages = utils.supressConsole(() => {
                a.set(1)
            })

            console.log(messages)

// [warn] [mobx] Observable ComputedValue@4 being read outside a reactive context
            expect(messages.length).toBe(0)
        } finally {
            mobx.configure({ observableRequiresReaction: false })
        }
    })

The observableRequiresReaction is triggered when hitting the following code path:
https://github.com/mobxjs/mobx/blob/master/src/core/derivation.ts#L93-L109

        case IDerivationState.POSSIBLY_STALE: {
            const prevUntracked = untrackedStart() // no need for those computeds to be reported, they will be picked up in trackDerivedFunction.

Due to the untrackedStart call

Related #916

Possible fix:
Make untrackedStart/untrackedEnd to have own globalState flag?

@urugator
Copy link
Collaborator

urugator commented Nov 8, 2019

Weird ... IIRC untracked shouldn't have any effect on allowStateReads which is the relevant flag for raising the error (rather than actual presence of derivation - that was the point of the rewrite...)

@Bnaya
Copy link
Member Author

Bnaya commented Nov 8, 2019

Maybe it's something else
I will look again

@urugator
Copy link
Collaborator

urugator commented Nov 8, 2019

It's not because of untracked.
What happens is, that you modify the observable, which notifies the computed, which has to recompute, which accesses the observable - at this point allowStateReads is false.
autorun is or isn't invoked later based on the computation result.
We have to allowStateReads during the recomputation.

Bnaya added a commit that referenced this issue Nov 8, 2019
@Bnaya Bnaya mentioned this issue Nov 8, 2019
3 tasks
@Bnaya
Copy link
Member Author

Bnaya commented Nov 8, 2019

PR up

@Bnaya
Copy link
Member Author

Bnaya commented Dec 15, 2019

published

@Bnaya Bnaya closed this as completed Dec 15, 2019
@lock
Copy link

lock bot commented Feb 13, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.

@lock lock bot locked as resolved and limited conversation to collaborators Feb 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants