generate local get/set functions instead of always using Ember.get/Ember.set

[machty]

This enhancement depends on https://github.com/ghempton/ember-script-rails/issues/1

But any use of the dot syntax that generates Ember.get/Ember.set (probably all of them) should generate

var get = Ember.get, set = Ember.set;

rather than just using the Ember. equivalents all over the place. This is consistent with the Ember.js codebase as well.

[machty]

p.s. I'd love to dig in with this stuff and learn about Redux, so if you've got bigger fish to fry, I'd love to take a stab at this, but I'd appreciate a push in the right direction as to where to start digging around.

[ghempton]

This is definitely on my radar. Thoughts on whether get and set should be official EmberScript keywords? Alternatively, I was thinking of using __get and __set (to avoid conflicts).

As far as the implementation is concerned, it should be relatively straightforward to modify the emberSet and emberGet methods inside of compiler.coffee to just use get and set instead of Ember.. Get and set also potentially need to be added as keywords inside of grammar.pegjs. Finally, all that remains is creating a header on each file that has get = Ember.get... etc. To do this, I would suggest looking at the CS.Program compilation rule inside of compiler.coffee and mimic what has been done with leadingComments.

PRs welcome :)

[machty]

What's the reasoning behind get and set as ES keywords? Dot syntax should cover all the cases, yes? Including the ones where you're not certain whether you're dealing with an Ember.Object or just a normal JS object?

[ghempton]

Just because eventually I want to use them to allow for separate getter and setter definitions for CPs. Still need to give that some thought, no need to make them keywords yet.

[machty]

Yeah, I also seem to remember Ember core wanting to rethink the getter/setter syntax. Might make sense to hold off on that til they have some idea.

On Mon, Dec 24, 2012 at 1:37 AM, Gordon L. Hempton <notifications@github.com

wrote:

Just because eventually I want to use them to allow for separate getter and setter definitions for CPs. Still need to give that some thought, no need to make them keywords yet.

— Reply to this email directly or view it on GitHubhttps://github.com/ghempton/ember-script/issues/2#issuecomment-11655190.

Alex Matchneer Co-Founder and Lead Dev Useful Robot LLC http://usefulrobot.io (614) 395-8832

[machty]

While it's on my mind, what would you think of the following getter setter syntax:

class Foo
  +computed
  myProp: -> 4
  myProp: (val) -> ...

  # Or if we wanted a further indication of setting, go the ruby route:
  myProp=: (val) -> ...
  myProp: (=val) -> ...

Basically, the +computed annotation would grant an exception from the unique property rule in CS to allow for a 0-arity getter and 1-arity setter. The order of getter/setter wouldn't matter, only that +computer comes right before one of them and that the two are adjacent (unnecessarily restrictive?)

Also, what's the syntax on native property setting with @? @*.foo = 4 ?

[ghempton]

Hmm, the arity approach is interesting, I think I need to sleep on it. One reason why the get / set approach is attractive is that it has been proposed for es6 https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Operators/get?redirectlocale=en-US&redirectslug=JavaScript%2FReference%2FOperators%2FSpecial%2Fget

@*. should work.

[machty]

I'm starting to wonder if Redux is at the point that it can accommodate this fix, but I'm open for suggestions. It seems like the right place for this fix as to add emberGet and emberSet to the helpers hash, which generates the necessary code for helpers if they end up getting used in the code. The only problem is that while the original CoffeeScript declares helpers via var __whatever = function() {...}, CSR seems to make full function declarations, and doesn't seem to support the var declaration approach, which is kinda what we need to set var get = Ember.get. Do you know if that's something they might be planning to address?

[ghempton]

After taking a cursory look at CSR's helper code, I think it might actually be exactly what we want. We could just add something like the following to the helper hash:

helpers:
  get: ->
    new JS.VariableDeclaration new helperNames.get, memberAccess(new JS.Identifier('Ember'), 'get')

Then just replace the emberGet calls inside the compiler to helpers.get (and same for set).

[ghempton]

It looks like CSR does use the declarative approach, the helpers all return declarations and literal name of the declaration is used in a call expression when the helper is invoked.

[machty]

Still getting stuck on this. Tried the VariableDeclaration approach (with the necessary nested VariableDeclarators), but that just seems to insert the declaration after the call is made to the automatically-generated get. I was also looking for a way to modify the declarationsNeeded function, which is in charge of putting all the declarations at the top, but it only handles declaration, and not the assignments. If I were to dig deeper and make it capable of assignment, I'd basically be solving the whole problem of CSR using the declarative approach, and maybe that needs to be done, but seems like I might be going about things the wrong way. Again, I also just can't really tell if the declarative approach is temporary (which would arguably be an easier way to do things given the way function declarations are taken into account before the rest of the code is evaluated) or whether in a few weeks they'll have switched it over to the classic CoffeeScript approach, but I'd definitely like to hear your thoughts.

[ghempton]

Thanks for putting time into this during the holidays! I will put some brain power into this tomorrow, but I don't know the code well enough off-hand to help any further. My guess is that CSR won't change this in the near future, but maybe we could get @michaelficarra to comment?

[michaelficarra]

The helper functions will be defined using function declarations for the foreseeable future. @ghempton: is that what you were wondering?

[ghempton]

@machty, I just submitted a PR containing my attempt at this.

@michaelficarra I think the confusion stems from the compilation rule for CS.Program putting helpers at the end of the block rather than at the beginning. Is this intentional? I notice that using an unshift instead of a push for enabledHelpers still results in all tests passing.

[michaelficarra]

Ah, I see. And you want to put an assignment in the helper list that (since it's an assignment) won't be evaluated until the end of the program. I think all helpers should be function declarations. Either way, their position shouldn't matter. They will be staying at the bottom.

In your case, you can define the helpers like this:

function get(){ get = Ember.get; return get.apply(this, arguments); }
function set(){ set = Ember.set; return set.apply(this, arguments); }

edit: added even better solution

[machty]

That would defeat the whole purpose of this fix, which is to produce code that mimics the pattern of the Ember code base, where a get and set var are declared and assigned to Ember.get and .set to cut down on unnecessary member accessing and function call overheard.

On Thursday, December 27, 2012, Michael Ficarra wrote:

Ah, I see. And you want to put an assignment in the helper list that (since it's an assignment) won't be evaluated until the end of the program. I think all helpers should be function declarations. Either way, their position shouldn't matter. They will be staying at the bottom.

In your case, you can do this:

function get(){ return Ember.get.apply(this, arguments); }function set(){ return Ember.set.apply(this, arguments); }

— Reply to this email directly or view it on GitHubhttps://github.com/ghempton/ember-script/issues/2#issuecomment-11711574.

Alex Matchneer Co-Founder and Lead Dev Useful Robot LLC http://usefulrobot.io (614) 395-8832

[ghempton]

@michaelficarra that makes sense, thanks for the input. The difference for our use case is that the purpose of moving this to a helper would be performance. The solution you propose would actually incur more overhead and would probably be better for us to leave it inline.

So @machty, we have two options for EmberScript:

1. Move the helpers to the beginning of the generated code and use an assignment (what my PR does).
2. Don't use the helper code and instead add the get and set assignments to the top of every file (similar to leading comments).

Thoughts?

[ghempton]

Oh interesting, I just noticed @michaelficarra's edited solution actually solves this by re-defining get after the first invocation. Still feels like we are trying to hack this in somewhere it shouldn't be. Maybe we should go for option 2.

[michaelficarra]

3. partition the helpers by helper.instanceof JS.FunctionDeclaration, pushing the declarations and unshifting anything else (like assignments).

I'd go with option 3 (and accept a pull request to CSR if you're willing to backport it :grin:).

[ghempton]

@michaelficarra that sounds like the best option and I'm certainly willing to get this into CSR. I can take a stab at this later tonight unless @machty wants to take it.

[machty]

I'd love to take a stab at it if there are more productive things you guys can do in the meantime. Hope to have something in by tonight.

(p.s. @michaelficarra can you recommend some literature on writing Haskell-based compilers? I figured it had to be an influence based on a cursory glance of functional-helpers.coffee)

[ghempton]

@machty go for it!

[machty]

Merry Christmas: https://github.com/michaelficarra/CoffeeScriptRedux/pull/129

[ghempton]

@machty nice! Once this is merged in to CSR, I will rebase my PR and we should be good!

[ghempton]

Should work now! Thanks all!