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

Improve type safety of name global variable #15424

Closed
falsandtru opened this issue Apr 27, 2017 · 5 comments
Closed

Improve type safety of name global variable #15424

falsandtru opened this issue Apr 27, 2017 · 5 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@falsandtru
Copy link
Contributor

falsandtru commented Apr 27, 2017

name global variable is still assignable to string type. So name is still unsafe. name: string | undefined works better than current definition name: never.

TypeScript Version: master

Code

const n1: string = <never>name; // pass
const n2: string = <string | never>name; // pass
const n3: string = <string | undefined>name; // error, expected

Expected behavior:

declare const name: string | undefined;

Actual behavior:

declare const name: never;
@falsandtru
Copy link
Contributor Author

related: #9850 @DanielRosenwasser

@falsandtru
Copy link
Contributor Author

@mhegazy @DanielRosenwasser Can you please also review this?

@mhegazy
Copy link
Contributor

mhegazy commented May 11, 2017

@DanielRosenwasser any thoughts

@mhegazy
Copy link
Contributor

mhegazy commented May 12, 2017

The resons we made it never was that ppl were stepping on it by mistake..

class C {
    name: string;
    method() {
        doSomething(name); // not this.name
    }
}

```
by making it `never` at least you will get an error and you will go back and check the source of the declaration. 

`string | undefined` will put us back where we started for most users.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Aug 15, 2017

as noted in #15424 (comment), never was added for a specific reason. It is much more common for users to use the global name by mistake vs intentionally. this change was meant to alert users in these cases; unfortunately it breaks other cases, but we believe this gets the 90% case right. for other cases, window.name.

@mhegazy mhegazy closed this as completed Aug 15, 2017
@mhegazy mhegazy added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Needs Investigation This issue needs a team member to investigate its status. labels Aug 15, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants