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

Fix 9363: passing undefined or null to parameter destructuring #9425

Closed
wants to merge 2 commits into from

Conversation

yuit
Copy link
Contributor

@yuit yuit commented Jun 29, 2016

Fix #9363
This probably should be ported to release-2.0 as well.
Also, technically, it is actually fine to pass undefined or null if the --target is es5 (at least for the empty destructing case as we won't be emit the destructuring in the body) I think it is more consistent to error regardless of the --target. let me know if you think it should respect the target more

@shovon
Copy link

shovon commented Jun 29, 2016

I'll admit, I was a bit careless when I wrote my comment in #9363.

I think my comment should be a part of a broader discussion about nullability of destructured parameters. I understand non-nullable types are slated for TypeScript 2.0, based on #185.

Maybe destructured parameters should be a non-nullable type by default?

@yuit
Copy link
Contributor Author

yuit commented Jun 29, 2016

@shovon @DanielRosenwasser and I discussed off-line, turned out there are some cases that we will have to consider

function foo({} = undefined) {}
foo(undefined)  // this should be an error as the initializer will get evaluted to be undefined

We think it will be a good idea to have a non-nullable type for destructuring. There are also this case that should be an error

var {} = undefined;
var [] undefined;

@yuit yuit closed this Jun 29, 2016
@yuit
Copy link
Contributor Author

yuit commented Jun 29, 2016

We should use slightly different approach to sovle the problem. Track the issue in #9429

@yuit yuit deleted the fixPassingToParameterDestructuring branch June 29, 2016 20:37
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants