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

Deps extensions #193

Closed
wants to merge 11 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@tmeasday
Contributor

tmeasday commented Jun 19, 2012

Hey,

I've created a little package that adds a couple of functions to Meteor.deps to abstract out some boiler plate code that I've found I've had to write a lot:

 Meteor.deps.add_reactive_variable(object, name, default_value)

Which adds three functions to object: object.name(), object.name.set() and object.name.equals() which behave reactively. Much like the session (see below). And:

 Meteor.deps.await(test_fn, cb)

Which will set up a context to repeatedly test test_fn and then call cb when it succeeds. There's more details and an example in the file.

NOTE on session.js:

I found that I was pretty much rewriting a lot of code from the session package, so I refactored session to use deps-extensions. I've added some basic tests too.

@TomWij

This comment has been minimized.

Show comment
Hide comment
@TomWij

TomWij Jun 19, 2012

Contributor

Watch out with using name (See link in deberg's answer), I don't know whether this applies to this code, but since I have a feeling that this is to be used client side from a first view on it I believe this can affect your code to not function properly on some browsers or some browser versions.

Contributor

TomWij commented Jun 19, 2012

Watch out with using name (See link in deberg's answer), I don't know whether this applies to this code, but since I have a feeling that this is to be used client side from a first view on it I believe this can affect your code to not function properly on some browsers or some browser versions.

@tmeasday

This comment has been minimized.

Show comment
Hide comment
@tmeasday

tmeasday Jun 20, 2012

Contributor

Ahh, thanks Tom but name was just a placeholder above. Perhaps we'd need to alert the user if they tried to use a 'reserved' name for a reactive variable. Not sure.

Contributor

tmeasday commented Jun 20, 2012

Ahh, thanks Tom but name was just a placeholder above. Perhaps we'd need to alert the user if they tried to use a 'reserved' name for a reactive variable. Not sure.

@possibilities

This comment has been minimized.

Show comment
Hide comment
@possibilities

possibilities Aug 9, 2012

Contributor

Love this! Merge me!

Contributor

possibilities commented Aug 9, 2012

Love this! Merge me!

@tmeasday

This comment has been minimized.

Show comment
Hide comment
@tmeasday

tmeasday Sep 16, 2012

Contributor

Note: @joshrtray's #272 probably is a cleaner way to do this (with ReactiveVar and ReactiveDict as first class objects); however, I'd like to point out a couple of things from this PR that might be worth retaining:

  1. I add a test suite for the session and for the extensions. Probably work keeping + extending.
  2. I'd also like to advocate for helpers in the other direction (setting up + maintaining reactive contexts). In this PR it's just await and await_once, but there are other obvious useful functions.

Rather than change this pull request, I'll just keep updating my smart package (https://github.com/tmeasday/meteor-deps-extensions/), which I should note I use in almost all of my other smart packages.

Contributor

tmeasday commented Sep 16, 2012

Note: @joshrtray's #272 probably is a cleaner way to do this (with ReactiveVar and ReactiveDict as first class objects); however, I'd like to point out a couple of things from this PR that might be worth retaining:

  1. I add a test suite for the session and for the extensions. Probably work keeping + extending.
  2. I'd also like to advocate for helpers in the other direction (setting up + maintaining reactive contexts). In this PR it's just await and await_once, but there are other obvious useful functions.

Rather than change this pull request, I'll just keep updating my smart package (https://github.com/tmeasday/meteor-deps-extensions/), which I should note I use in almost all of my other smart packages.

@tmeasday

This comment has been minimized.

Show comment
Hide comment
@tmeasday

tmeasday Dec 13, 2012

Contributor

Oh, I guess this isn't needed any more...

Contributor

tmeasday commented Dec 13, 2012

Oh, I guess this isn't needed any more...

@tmeasday tmeasday closed this Dec 13, 2012

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