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

now() returns not a relevant time if accessed from outside a reactive context #40

Closed
rasentry opened this issue Feb 16, 2017 · 12 comments
Closed

Comments

@rasentry
Copy link

When I use now in computed value and use it outside reaction it returns not relevant time.
see https://jsfiddle.net/pwcv8xsu/

@mattruby
Copy link
Member

See this example: https://jsfiddle.net/pwcv8xsu/1/

Does that help you out?

@rasentry
Copy link
Author

rasentry commented Feb 16, 2017

@mattruby No, because getter "time" is used not only inside reaction(autorun).

@mattruby
Copy link
Member

@rasentry we're getting a little funky here, but you'll need to use keepAlive
See this example: https://jsfiddle.net/pwcv8xsu/3/

@mattruby
Copy link
Member

Mobx will try to clean up computeds that are not being used in a reaction. So we're going to tell mobx not to do that with keepAlive. Note that keepAlive will return a disposer. You'll want to run that when/if you're done with that time.

@rasentry
Copy link
Author

As I think it is not appropriate solution, because I need to take care about disposer.
In my project I use code from example from here https://mobx.js.org/refguide/extending.html
and there is no need disposer.

@mattruby
Copy link
Member

mattruby commented Feb 16, 2017 via email

@rasentry
Copy link
Author

rasentry commented Feb 16, 2017

Yes I use my gette inside and outside reaction. Inside reaction it works as designed, but when no reaction on this getter it returns old (cached) value.

@rasentry
Copy link
Author

I think the problem is here https://github.com/mobxjs/mobx-utils/blob/master/src/from-resource.ts#L106
there is always returns cashed value, which updates only in reaction.
In example https://mobx.js.org/refguide/extending.html
there is check

if (this.atom.reportObserved()) {
            return this.currentDateTime;
        } else {
            // apparantly getTime was called but not while a reaction is running.
            // So, nobody depends on this value, hence the onBecomeObserved handler (startTicking) won't be fired
            // Depending on the nature
            // of your atom it might behave differently in such circumstances
            // (like throwing an error, returning a default value etc)
            return new Date();
        }

@mattruby
Copy link
Member

mattruby commented Feb 16, 2017 via email

@mweststrate
Copy link
Member

mweststrate commented Feb 16, 2017 via email

@rasentry
Copy link
Author

@mweststrate As a temporary solution it is good, but I think now() should work like a "normal" Date.now(), isn't it?

@mweststrate mweststrate changed the title now() returns not relevant time. now() returns not a relevant time if accessed from outside a reactive context Jun 13, 2017
@mweststrate
Copy link
Member

Pushed a fix, will be part of next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants