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

Local reactive template state #3561

Closed
wants to merge 3 commits into from
Closed

Local reactive template state #3561

wants to merge 3 commits into from

Conversation

@stubailo
Copy link
Contributor

stubailo commented Jan 26, 2015

You can now use Template.instance().state to store local reactive data for your template. There is also a Template.state built-in helper that you can use to access it without defining a new helper.

to do:

  1. Add serialization hooks for migration
  2. Refactor reactive dict to not always serialize things
@stubailo
Copy link
Contributor Author

stubailo commented Jan 27, 2015

Still need to add tests for migration.

@stubailo
Copy link
Contributor Author

stubailo commented Jan 27, 2015

Also add test to check that the keyword Template.state is indeed reactive when you change the argument and when you set something on state.

'dynamic': 'Template.__dynamic'
'dynamic': 'Template.__dynamic',

'state': '_.bind(view.templateInstance().state.get, view.templateInstance().state)'

This comment has been minimized.

Copy link
@avital

avital Jan 27, 2015

Contributor

The resulting JS that gets compiled from templates should look clean. If this dance is what we need, define a utility function in the spacebars package (where the runtime for Spacebars compiled templates live).

This comment has been minimized.

Copy link
@dgreensp

dgreensp Jan 27, 2015

Contributor

Or 'state': 'Template.__state'

This comment has been minimized.

Copy link
@stubailo

stubailo Jan 27, 2015

Author Contributor

I like this idea, except wouldn't I need to pass the view in as an argument?

This comment has been minimized.

Copy link
@dgreensp

dgreensp Jan 27, 2015

Contributor

Hmm, don't we have a dynamic variable for that? Template.instance()

This comment has been minimized.

Copy link
@stubailo

stubailo Jan 28, 2015

Author Contributor

Good idea!

@Slava Slava added the Project:Blaze label Jan 27, 2015
@dgreensp
Copy link
Contributor

dgreensp commented Jan 27, 2015

I haven't finished reading this yet, but what is the template syntax? {{Template.state "foo"}}?

We don't have to block on having an answer for this, but the obvious question is how to make that better so that you can use state in, say, an argument to a helper. Something closer to state.foo.

@stubailo
Copy link
Contributor Author

stubailo commented Jan 28, 2015

I don't think you would use state in an argument to a helper usually. I think Template.state "foo" is the best we can do until we improve scoping mechanics.

@dalerka
Copy link

dalerka commented Jan 30, 2015

Hey guys! My apologies if this is not relevant here (looking forward for changes described by @stubailo):

To keep the Meteor API as succinct as possible (easier for learning and using) perhaps it is better to have one "name" for some functionality instead of two, i.e to have this.state and Template.state to set and get some values instead of having another this.vars (which is less semantic than this.state IMHO). Excerpt from @stubailo 's blog post:

this.vars.setDefault(“selectedDay”, new Date());
... 
{{Template.state.selectedDay}}
@stubailo
Copy link
Contributor Author

stubailo commented Jan 30, 2015

@dalerka the inconsistency was a typo on my part in the blog post. After talking a little with @debergalis we are currently thinking of calling this feature "vars" as opposed to "state" because "state" seems too jargon-ey. Do you have any ideas for other names that will be easy to understand for new Meteor developers?

@dalerka
Copy link

dalerka commented Jan 31, 2015

@stubailo ok, thanks. If the anticipated API ("vars") is intended to help with managing the Template's state of variables then vars seems simple and easy to get used to. But if you guys have some ideas that this API will serve other purposes beyond stated above, then perhaps there could be another name. I hope it won't cause much confusion when talking about such states of Template as rendered or created etc. The alternative names that I can think of are store or temp.

@lourd
Copy link

lourd commented Feb 3, 2015

This hackpad that @gschmidt started a while back has a lot of ideas about API names, scoping, and general object-oriented patterns for Templates/Components that could be good to take a look at again

@stubailo
Copy link
Contributor Author

stubailo commented Feb 4, 2015

Looks like there Geoff decided on fields where we currently have vars (the *.define syntax is a whole different thing that we are also potentially thinking about).

I'm not sure that fields is clearer than vars or state.

@dgreensp
Copy link
Contributor

dgreensp commented Feb 4, 2015

I brought up "fields" and Matt said it made him think of text fields, and I agree.

As a bit of trivia, the name "field" for reactive state originally dates back to when we were thinking of calling the current template instance Meteor.form!

@stubailo
Copy link
Contributor Author

stubailo commented Feb 4, 2015

Wow, interesting. Looks like dynamic variables were all the rage back then.

@dburles
Copy link
Contributor

dburles commented Feb 13, 2015

+1 for naming it state. The conversations here (and elsewhere) I see it naturally referred to as the "Reactive state" of a template. vars on the other hand makes me think of ReactiveVar, which it is not. Especially if it's going to be clear in the docs that state is a ReactiveDict.

@stubailo
Copy link
Contributor Author

stubailo commented Feb 13, 2015

@dburles personally I agree with you that state is more specific. I'm not really sure that vars is less jargon-y, especially since they are really just dictionary keys and not variables in their own right. I think it's right that we can just say "this.state is a reactive dictionary where templates can keep any local UI state, for example the current tab open or the state of a dropdown menu."

I wonder who we could ask to actually find out which one is better. React calls this thing state, so maybe it's better to be less confusing by using a similar word for a similar thing. @debergalis what do you think?

@debergalis
Copy link
Member

debergalis commented Feb 14, 2015

okay, sure.

@stubailo stubailo force-pushed the template-state branch from 5756bcf to 2b9d94f Apr 2, 2015
@mitar
Copy link
Collaborator

mitar commented Apr 20, 2015

I would like to link to #3590. I think ReactiveDict needs some memory cleanup support before being useful here.

@Maxhodges
Copy link
Contributor

Maxhodges commented May 22, 2015

would hate to see template.state repeat the same design mistake we see in ReactiveDict. Maybe now is a chance to do it right?

See #4100 (comment)

@aadamsx
Copy link

aadamsx commented May 23, 2015

Completely agree with @Maxhodges and @mitar on this, please give ReactveDict some love.

@stubailo
Copy link
Contributor Author

stubailo commented May 23, 2015

Can we keep this thread on topic, please?

@aadamsx
Copy link

aadamsx commented May 23, 2015

I understand this probably isn't the right spot to voice concerns/feedback about meteor implementations/features.

Sorry about that.

To keep this a clean room, is there a spot for discussion with the MDG on topics such as ReactiveDict? Or should someone like @Maxhodges or myself open a issue here to start a discussion? (I assume feedback and discussion are welcomed by the MDG on any topic)

EDIT: While writing this I saw #4438 and your feedback on #4100. Thanks.

@stubailo
Copy link
Contributor Author

stubailo commented Jun 30, 2015

Probably not going to happen soon, and this PR doesn't have that much work in it anyway.

@stubailo stubailo closed this Jun 30, 2015
@ghost
Copy link

ghost commented Jun 30, 2015

@stubailo Can you give some insight why this feature is not merged? Would be interesting for me.

@stubailo
Copy link
Contributor Author

stubailo commented Jun 30, 2015

I think I didn't actually add enough value to add a new feature - ideally, this wouldn't just be the same as doing:

Template.x.onCreated(function () {
  this.state = new ReactiveDict();
});

I would have to build a lot more stuff to make it better, and I don't have time. So I'm explicitly abandoning this PR.

@mitar
Copy link
Collaborator

mitar commented Jun 30, 2015

I use something similar with Blaze Components and reactive fields.

@ghost
Copy link

ghost commented Jul 1, 2015

Ok. I use ReavtiveVar right now for this. Works for me. This way it is explicit in one place what reactive state the template has. The downside is a bit boilerplate code for the helpers.

var template = Template.x;

template.onCreated(function () {
  this.myReactiveStateVar1 = new ReactiveVar();
  this.myReactiveStateVar2 = new ReactiveVar();
});

template.helpers({
  myReactiveStateVar1: function () {
    return Template.instance().myReactiveStateVar1.get();
  ),

  myReactiveStateVar2: function () {
    return Template.instance().myReactiveStateVar2.get();
  )
});
@mitar
Copy link
Collaborator

mitar commented Jul 13, 2015

Yea, in Blaze Components this becomes much simpler. :-) You do not have all this boilerplate:

var ExampleComponent = BlazeComponent.extendComponent({
  onCreated: function () {
    this.myReactiveStateVar1 = new ReactiveField();
    this.myReactiveStateVar2 = new ReactiveField();
  }
}).register('ExampleComponent');
@rclai
Copy link

rclai commented Jul 13, 2015

@mitar Yes, it is nice to be able to declare it and not have to write a helper to access it. We can just start using it in the template!

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

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.