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

Type narrowing doesn't work with mutable variables in closures #23776

Closed
falsandtru opened this issue Apr 30, 2018 · 14 comments · Fixed by #56908
Closed

Type narrowing doesn't work with mutable variables in closures #23776

falsandtru opened this issue Apr 30, 2018 · 14 comments · Fixed by #56908
Labels
Duplicate An existing issue was already created

Comments

@falsandtru
Copy link
Contributor

Includes arguments.

TypeScript Version: 2.9.0-dev.201xxxxx

Search Terms:

Code

declare const a: {} | void;
if (a instanceof Object) a; // {}
if (a instanceof Object) () => a; // {}
declare let b: {} | void;
if (b instanceof Object) b; // {}
if (b instanceof Object) () => b;

Expected behavior:

if (b instanceof Object) () => b; // {}

Actual behavior:

if (b instanceof Object) () => b; // void | {}

Playground Link:

Related Issues:

@falsandtru
Copy link
Contributor Author

Seems like it also breaks colorization.

@ghost
Copy link

ghost commented Apr 30, 2018

Duplicate of #9998
Could you explain the problem with colorization?

@ghost ghost added the Duplicate An existing issue was already created label Apr 30, 2018
@falsandtru
Copy link
Contributor Author

Indeed.

On the colorization, the last return keyword is white, should be blue.

    if (process instanceof Coroutine) return this.register(
      name,
      {
        init: state => state,
        main: (param, state) =>
          (process as Coroutine<R, R, P>)[Coroutine.port].send(param)
            .then<Supervisor.Process.Result<R, S>>(({ value: reply, done }) =>
              done
                ? void this.kill(name, undefined) ||
                  (process as Coroutine<R, R, P>).then<Supervisor.Process.Result<R, S>>(reply =>
                    [reply, state])
                : [reply, state,]),
        exit: reason => void process[Coroutine.terminator](reason),
      },
      state,
      reason);
    return super.register(name, process, state);

https://github.com/falsandtru/spica/blob/v0.0.167/src/supervisor.es2018.ts#L29

@ghost
Copy link

ghost commented Apr 30, 2018

@falsandtru What editor? I see colorized return when pasting that example into vscode. (With or without the latest grammar installed.)

@falsandtru
Copy link
Contributor Author

Hm, I'm using VSCode v1.22.2 on Win10.

@ghost
Copy link

ghost commented Apr 30, 2018

What's your color scheme?

@falsandtru
Copy link
Contributor Author

Dark (Visual Studio).

@ghost
Copy link

ghost commented Apr 30, 2018

I'm on windows, on the latest vscode-insiders, with the dark color scheme and latest grammar installed, and both returns are colored blue:

color

@falsandtru
Copy link
Contributor Author

Oh No! Please open and check https://github.com/falsandtru/spica/blob/v0.0.167/src/supervisor.es2018.ts#L29 via vscode!

@ghost
Copy link

ghost commented Apr 30, 2018

Looks like the error is only present in vscode non-insiders without the latest grammar installed.

@falsandtru
Copy link
Contributor Author

This is my screenshot.

image

@falsandtru
Copy link
Contributor Author

falsandtru commented Apr 30, 2018

Looks like the error is only present in vscode non-insiders without the latest grammar installed.

I see, I wait for the next release. Currently, this is not a serious problem for me.

@ghost
Copy link

ghost commented Apr 30, 2018

You can also open the extensions sidebar and search for "latest typescript and javascript grammar".

@falsandtru
Copy link
Contributor Author

Resolved, thanks for letting me know!

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant