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

Defaults for base types #23

Open
lukescott opened this issue Jun 11, 2015 · 16 comments · May be fixed by #37
Open

Defaults for base types #23

lukescott opened this issue Jun 11, 2015 · 16 comments · May be fixed by #37

Comments

@lukescott
Copy link
Owner

We need to put somewhere in this spec that base values provide defaults if not specified.

For example, the following code:

function doSomething(s string, n number, b boolean) {
}
var obj = {
  s string,
  n number,
  b boolean
};

Should desugar to:

function doSomething(s = "", n = 0, b = false) {
}
var obj = {
  s: "",
  n: 0,
  b: false
};

That way you don't end up with undesired behavior with undefined.

@lukescott lukescott changed the title Defaults for base values Defaults for base types Jun 11, 2015
@Naddiseo
Copy link
Collaborator

For parameters, variable declarations, and destructuring that makes sense. Because:

function doSomething(s, n, b) {
    if (!s) { /* s wasn't passed*/ }
    if (!n) { /* n wasn't passed */ }
    if (!b) { /* b either wasn't passed, or is false */ }
}
doSomething(); // equal to doSomething(undefined, undefined, undefined)

// with defaults
function doSomething2(s = "", n = 0, b = false) {
    if (!s) { /* s wasn't passed*/ }
    if (!n) { /* n wasn't passed */ }
    if (!b) { /* b either wasn't passed, or is false */ }
}
doSomething2(); // equal to doSomething2(undefined, undefined, undefined)

Basically, ToBoolean(param) is the same with the defaults as without.

However, your second example is incorrect (and might be invalid sytnax?):

var obj = {
  s string,
  n number,
  b boolean
};

// desugars to:
var obj = {
   s: s,
   n: n,
   b: b
};

I'm not sure what to do about that. One possibility is:

// desugared:
var obj = {
     s: (s !== void 0 ? s : ""),
     n: (n !== void 0 ? n : 0),
     b: (b !== void 0 ? b : false),
};

I'm on the fence regarding this, we'll have to see what other people have to say about it.

@lukescott
Copy link
Owner Author

Oh I forgot about this new ES6 feature:

var s;
var n;
var b;

var obj = {
    s,
    n,
    b
};

// goes to

var obj = {
    s: s,
    n: n,
    b: b
};

Without that it wouldn't be a problem... I wonder if providing a type-hint should be different than the above. Because this would not make any sense:

var s boolean;
var n number;
var b string;

var obj = {
  s string,
  n number,
  b boolean
};

The types wouldn't match. So providing a type-hint should probably be a shorthand for:

var obj = {
  s string: "",
  n number: 0,
  b boolean: false
};

It's kind of an odd situation. But I think that makes sense to me.

@Naddiseo
Copy link
Collaborator

I think I disagree. I think the TypeHints should always be on the LHS of an =. Thus:

var obj Object = {
    s, n , b
};

@lukescott
Copy link
Owner Author

Not sure what your Object example has to do with it. Just to make sure, I'm talking about the properties inside the object (string number and boolean).

If we're on the same page with that:

So are you saying both of these should be invalid?

var obj = {
  s string,
  n number,
  b boolean
};
var obj = {
  s string: "",
  n number: 0,
  b boolean: false
};

If so, here's the issue: There are a lot of parallels between an a JavaScript object and a class:

class Foo {
  doThis() {

  }
}
var foo = {
  // short-hand method is valid in an object too!
  doThis() {

  }
};

And if a class ends up using a : instead of a = (to stay consistent with {}), then the LHS of an = rule no longer applies:

class Foo {
  s string: ""
  n number: 0
  b boolean: false
}

Also, people still use JavaScript objects for class-like things:

var Foo = Object.freeze({
  // add some type hints without rewriting code:
  s: "", // -> s string,
  n: 0, // -> n number,
  b: false, // -> b false,

  init: function() {
    return Object.create(this);
  },

  doThis: function() {}
});

var foo = Foo.init();

So being able to add type-hints to existing code would be very helpful. Flow assumes that s: "" should always be a string, but we're saying if a type hint isn't provided it's mixed. Allowing type hints on {} would allow someone to do a find a replace of :"", -> string,.

@Naddiseo
Copy link
Collaborator

I see what you're saying. I'm just really on the fence about it.

So are you saying both of these should be invalid?

var obj = {
  s string,
  n number,
  b boolean
};
var obj = {
  s string: "",
  n number: 0,
  b boolean: false
};

At the moment, I'm thinking they're both syntactically valid.

For consistency with classes, it makes a lot of sense.

var x Hint = 1; // makes sense
var x Hint = {}; // makes sense
class A {
   x Hint:  1; // Makes sense
};
var O Hint = { // makes sense, consistent with other vars
   x Hint: 1, // makes sense, has code smell, but same as class (aside from semicolon vs comma)
};
var O Hint = {
   X Hint, // This throws up warning bells for me, depending on the semantics.
};

I have a problem with the default expansion:

var x bool = 1;
var o = { x string };
// is shorthand for
var o = {x string: x };
// NOT
var o = {x string: "" };

In that case, I think the {x string: x} should throw a syntax error in the static semantics phase because of the type mismatch. Actually, does it throw an error at all? Because that's type checking, not hinting...

Allowing type hints on {} allow someone to do a find a replace of :"", -> string,.

Good point, except it should be made for = "" -> string

There's also this case we should consider:

let o = { A: undefined };
let { A string } = o; // What is the value of A?

// similarly:
function f(arg string = undefined) { }
f(); // what is arg?

@Naddiseo
Copy link
Collaborator

Another inconsistency with this, is usage with non-base types:

class A {}
let x A; // what's the default value here?

@lukescott
Copy link
Owner Author

I have a problem with the default expansion:

var x bool = 1;
var o = { x string };
// is shorthand for
var o = {x string: x };
// NOT
var o = {x string: "" };

See, I think there is a big difference between this:

var obj = {s};

And this:

var obj = {s string};

The first one is a short-hand of:

var obj = {s};

Which is assuming you have a var s somewhere. It should inherit the type from var s.

The second one, {s string}, seems like it should be:

var obj = {s:""};

I think adding a type-hint should negate that shortcut, especially since you won't be able to do that in a class.

Also, this is invalid:

var s number; // default;
var obj = {s string};

There is a type conflict there. That's why {s} and {s string} are very different. At least it seems that way to me.

If we decided that you can't put type hints on {} properties, you then have this:

var obj = {
  getValue() string {
    return "";
  }
};

A method on an object, which looks just like a method on class, can have a TypeHint on it. Why not a property at this point?

Another way to resolve the LHS = thing is to allow this, especially if class ends up using = for properties:

var obj = {
  s string = "" // require = with a typehint
};

The only thing I don't like about that is it's no longer JSON. But then again, JSON is very different than js objects:

  • JSON requires quotes around keys, js objects do not (I wish JSON didn't, honestly)
  • js objects can have methods

In that case, I think the {x string: x} should throw a syntax error in the static semantics phase because of the type mismatch. Actually, does it throw an error at all? Because that's type checking, not hinting...

We're just hinting & providing defaults. So yep, no error throwing :)

Another inconsistency with this, is usage with non-base types:

class A {}
let x A; // what's the default value here?

The default of any class (non-base type) is undefined, which is what it already is anyway. Defaults are only important for base types (primitives) because you could get undefined if you don't set a default. Having to explicitly set ""/0/false seems redundant anyway. You really can't provide a default for a class :).

@Naddiseo
Copy link
Collaborator

Okay, let's run with it as is. I have a feeling that people will complain about the {x string} expansion, but let's revisit that issue if/when it bites someone.

I'll try to get some of the static semantics done when I have some time.

Naddiseo pushed a commit that referenced this issue Jun 17, 2015
Disallow void generators.
Ensure setters are void. (closes #12)
Added `any` as a base type. (closes #14)
@Naddiseo Naddiseo linked a pull request Jun 17, 2015 that will close this issue
@Naddiseo
Copy link
Collaborator

What would the default type for any be, null/undefined?

@lukescott
Copy link
Owner Author

It would be undefined. That's the default when function arguments are omitted.

@Naddiseo
Copy link
Collaborator

I'm not sure this can be done for object literals. I'm reading through the syntax for the object initializers, and it assumes:

let o = { Identifier }; // It assume "Identifier" is the value, not the key
// desugars to:
let o = {}
o[%StringValue(Identifier)] = Identifier;

Making it desugar to modify the RHS of the assignment seems like a much bigger than we should be doing...

@Naddiseo Naddiseo self-assigned this Jun 17, 2015
@lukescott
Copy link
Owner Author

Isn't Identifier both the key and the value? I think in that case we have to assume any. We have no way of knowing what Identifier is.

However, something like this:

let o = { Identifier string }; // "Identifier" is key, not the value
// desugars to:
let o = {
  Identifier: ""
}
// not sure what goes here...:)

@Naddiseo
Copy link
Collaborator

My example is what the spec currently does: http://www.ecma-international.org/ecma-262/6.0/#sec-object-initializer-static-semantics-propname

What you specify in the initializer is what the value is, not what the key is.

@lukescott
Copy link
Owner Author

Right, what I'm saying is:

var Identifier = "foo";
// ES6 code:
var o = {Identifier};
// Desugars to ES5 code:
var o = {Identifier: Identifier};
// Which can be thought of as:
var o = {Identifier any: Identifier};
// logs as:
console.log(o); // {Identifier: "foo"}

Is different than:

var Identifier = "foo";
// ES6 code:
var o = {Identifier string};
// Desugars to ES5 code:
var o = {Identifier: ""};
// Which can be though of as:
var o = {Identifier string: ""};
// logs as:
console.log(o); // {Identifier: ""}
// Which means if you want to use "var Identifier" you need to do (explicitly):
var o = {Identifier string: Identifier}

Which would be consistent with class properties. I suppose class Foo {Identifier} could be valid too, which would be different than {Identifier}. I really wish {Identifier} wasn't a thing in ES6 :). Makes it so much harder...

@Naddiseo
Copy link
Collaborator

Yeah, I think I can get it to work by changing the semantic part of the
spec. I'll check when I get home.
On Jun 17, 2015 5:39 PM, "Luke Scott" [email protected] wrote:

Right, what I'm saying is:

var Identifier = "foo";// ES6 code:var o = {Identifier};// Desugars to ES5 code:var o = {Identifier: Identifier};// Which can be thought of as:var o = {Identifier any: Identifier};// logs as:console.log(o); // {Identifier: "foo"}

Is different than:

var Identifier = "foo";// ES6 code:var o = {Identifier string};// Desugars to ES5 code:var o = {Identifier: ""};// Which can be though of as:var o = {Identifier string: ""};// logs as:console.log(o); // {Identifier: ""}// Which means if you want to use "var Identifier" you need to do (explicitly):var o = {Identifier string: Identifier}

Which would be consistent with class properties. I suppose class Foo
{Identifier}, which would be different than {Identifier}. I really wish
{Identifier} wasn't a thing in ES6 :). Makes it so much harder...


Reply to this email directly or view it on GitHub
#23 (comment)
.

@Naddiseo
Copy link
Collaborator

@lukescott, Just made an attempt at allowing default values (#44).. and it's really messy. I definitely need help from someone who actually knows the spec more intimately than me.

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

Successfully merging a pull request may close this issue.

2 participants