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

expose current loop state #23

Merged
merged 4 commits into from Aug 31, 2015
Merged

expose current loop state #23

merged 4 commits into from Aug 31, 2015

Conversation

ghost
Copy link

@ghost ghost commented Aug 31, 2015

This patch exposes the current loop state as loop.state. I find myself repeatedly needing to read the loop state in many circumstances, so I usually create a state variable, but then I need to keep the loop state in sync with that variable and I need to pass around both objects all over the place. One recent example is to get browserify-hmr working well with main-loop, I need to trigger a loop update with the current state unchanged. With this patch loop.update(loop.state) will do the trick.

One caveat is that people might try writing to this value instead of calling loop.update() but I added a note to the docs to say not to do that.

If nothing else, you can cherry-pick 0b534b5 to get more API docs without the other state-exposing changes.

@juliangruber
Copy link

One caveat is that people might try writing to this value instead of calling loop.update() but I added a note to the docs to say not to do that.

we could use object.{{un,}freeze} if available to help preventing this

@yoshuawuyts
Copy link

Patch looks good. Agree with julian: .freeze() could be reasonable to enforce values to be read-only. Alternatively a getter could perhaps work?

var state = initialState
var loop = {
  target: target,
  update: update
}

Object.defineProperty(loop, 'state', {
  get: function () { return state }
})

return loop

@Raynos
Copy link
Owner

Raynos commented Aug 31, 2015

lgtm as is.

Raynos added a commit that referenced this pull request Aug 31, 2015
@Raynos Raynos merged commit 640d5b3 into Raynos:master Aug 31, 2015
@Raynos
Copy link
Owner

Raynos commented Aug 31, 2015

Note:

  • Object.freeze is too opinionated; we don't own the state and we cant freeze it.
  • ES5 getter's add complexity and performance penalties that are not worth it. We should trust users to not do bad things.

@Raynos
Copy link
Owner

Raynos commented Aug 31, 2015

Published v3.2.0

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

Successfully merging this pull request may close these issues.

None yet

3 participants