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

'it' is not defined when referenced only by object literals? #899

Closed
dk00 opened this issue Jun 7, 2016 · 6 comments
Closed

'it' is not defined when referenced only by object literals? #899

dk00 opened this issue Jun 7, 2016 · 6 comments
Labels

Comments

@dk00
Copy link
Contributor

dk00 commented Jun 7, 2016

->
  it.name
->
  it
  {it.name}
->
  {it.name}

compiles to:

// Generated by LiveScript 1.5.0
(function(it){
  return it.name;
});
(function(it){
  it;
  return {
    name: it.name
  };
});
(function(){
  return {
    name: it.name
  };
});

the third function has no argument

Another example:

t = ->
  (cb) ->
    it
// Generated by LiveScript 1.5.0
var t;
t = function(){
  return function(cb){
    return it;
  };
};
@WreckedAvent
Copy link

WreckedAvent commented Jun 8, 2016

More complicated than just the object literal, it's something to do with the shorthand.

->
  { name: it.name }

compiles to =>

(function(it){
  return {
    name: it.name
  };
});

Similarly,

-> { it }

compiles to =>

(function(){
  return {
    it: it
  };
});

@summivox
Copy link
Contributor

From code readibility point of view I don't like t = -> (cb) -> it implying it in the outermost func. IMHO it is intended for short and simple expression lambdas (that cannot be expressed with the more concise partially-applied operator syntax), not any arrow function that has a single param, and certainly not across function scopes.

I wish we could:

  • deliberately limit it inference to direct function scope
  • emit warning/error on nested it functions e.g. t = -> alert it; do -> alert it or example above.

@rhendric
Copy link
Collaborator

My reading of the it feature (which, modulo this bug, seems to agree with lsc) is that it always refers to the first argument of the innermost function; so -> (cb) -> it should have it == cb. A warning in cases like this might be nice, but should only be reserved for cases where it's very likely the programmer is confused, IMO.

(I have a fix for this bug that's part of a slightly larger unit of work; I'll be PRing it one of these days, either with that work or separately.)

@summivox
Copy link
Contributor

summivox commented Oct 25, 2016

@rhendric : I beg to differ. it === cb is aliasing. You already have a name for the first parameter of the inner arrow function; it is confusing to have a second implicit name for it. It does avoid the cross-scope problem (i.e. it is always local function scope).

My understanding (which of course does not agree with LSC master) had been:

the unnamed implicit single argument of the innermost function without an explicit parameter list

i.e. -> (cb) -> it compiles to (it) -> (cb) -> it. Note that I'm not advocating for this, nor do I write code like this -- I am trying to point out to part of the confusion.

As for a stricter syntax, I believe we should emit warning/error when:

  • it is found in a function with an explicit parameter list but not assigned anywhere in the lexical scope (honestly I would even remove the "but" clause to make it stricter)
  • a function that contains implicit it is nested in a function that also contains implicit it

Regardless I'm looking forward to your big chunk of change ;)

@rhendric
Copy link
Collaborator

Upon revisiting this, it appears that I had entirely imagined this aliasing feature! I thought that was how things worked already in some cases, but I'm quite happy that it isn't. For the record, I retract my ‘should’ above: -> (cb) -> it is already compiling exactly how I think it should (plus or minus a warning/error), with the it assumed to be a variable in an outer scope or on the global object.

I'll hold further opining on issuing warnings or errors for possibly confusing uses of it until that proposal gets its own issue. 😃

@dk00
Copy link
Contributor Author

dk00 commented Dec 20, 2016

name: it.name parses to:

  Obj
    Prop
      Key name
      Chain
        Var it
        Index .
          Key name

{it.name} compiles to the same, but parses differently:

  Obj
    Prop
      Key name
      Chain
        Key it
        Index .
          Key name

it should be Var, but the parser gives Key. Maybe we should convert the shorthand to the same AST?

that also does the same, as expected.
data = {that.name} if result compiles to:

if (result) {
  data = {
    name: that.name
  };
}

dk00 added a commit to dk00/livescript that referenced this issue Dec 20, 2016
@gkz gkz closed this as completed in 3b9421a Dec 26, 2016
gkz added a commit that referenced this issue Dec 26, 2016
Add test cases and fix implicit it in object shorthands (#899)
This was referenced Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants