Skip to content

Conversation

@Mouvedia
Copy link
Collaborator

This resolves the concern raised in #111 by being more explicit.
For convenience, the specification assumes that undefined is read-only.

This resolves the concern raised in #111 by being more explicit.
For convenience, the specification assumes that undefined is read-only.
@Mouvedia
Copy link
Collaborator Author

@ericelliott if it's merged remember to squash it.

@ericelliott
Copy link
Owner

Those are not necessarily equivalent. I would assume that if present, an optional param will be of type Type, whereas if you explicitly pass undefined, the param may be present, but wouldn't conform to type Type.

The difference is subtle, but particularly interesting with named parameter objects:

const options = {
  foo: undefined,
  bar: 'bar'
};

// myFunc({ foo?: String, bar: String }) => NewThing
const resultA = myFunc(options) // Type error. If supplied, `foo` must be a string.

const resultB = myFunc({ bar: 'baz' }); // OK because `foo` isn't supplied.

We should address this in the spec, but I'm not sure which way to go on it. Thanks for bringing it up. =)

@Mouvedia
Copy link
Collaborator Author

Mouvedia commented Nov 22, 2016

@ericelliott the default is only used when nothing is passed. Arguments—in contrast with parameters—is what counts in that case. There's no argument when the default is used. There's nothing wrong with my addition in that regard. Your example has nothing to do with my addition.

e.g.

test = function (a, b = undefined) { console.dir(arguments); console.log(arguments.length); };
test(1);

@ericelliott
Copy link
Owner

Arguments—in contrast with parameters—is what counts in that case.

That's not really relevant. The runtime type checker is never going to see the parameter default value. Instead, it's going to essentially do this:

if (Object.hasOwnProperty(options.foo)) {
  if (isType(options.foo, expectedType)) return true;
  return false;
}
return true;

@Mouvedia
Copy link
Collaborator Author

Mouvedia commented Nov 22, 2016

I think we are talking about different things.

I am talking about

// (param1: Type, param2 = undefined: Type) => ReturnType
example1(arg1);

and you are talking about

// (param1: Type, param2: Type) => ReturnType
example2(arg1, undefined); // type error

@ericelliott Are you saying that we can't tell the difference between example(arg1); and example(arg1, undefined);?

If what you are saying were true it would mean that the type covers the argument and the default which wouldn't let us have a default of a different type than the type declared. @ericelliott Is that what you are scared of?

We don't need to set a separate type for the default because it is always implicit since defaults' values are known before hand.

@ericelliott
Copy link
Owner

ericelliott commented Nov 22, 2016

and you are talking about

No, I'm not. Re-read the logic.

No, I'm not saying we can't tell the difference between example(arg1); and example(arg1, undefined); I'm saying we can, and we can even tell the difference between example({ arg1 }) and example({ arg1, arg2: undefined }) and that the difference is significant.

@Mouvedia
Copy link
Collaborator Author

Mouvedia commented Nov 22, 2016

Ill quote myself because it seems that you have missed my question:

If what you are saying were true it would mean that the type covers the argument and the default which wouldn't let us have a default of a different type than the type declared. @ericelliott Is that what you are scared of?

@Mouvedia
Copy link
Collaborator Author

I am closing this in favor of #118.

@Mouvedia Mouvedia closed this Dec 19, 2016
@Mouvedia Mouvedia deleted the mouvedia-optional branch December 19, 2016 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants