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

feat: support typeof on #private Fields #2174

Merged
merged 3 commits into from
Apr 11, 2022

Conversation

magic-akari
Copy link
Contributor

TypeScript 4.7 now allows us to perform typeof queries on private fields.

class Container {
    #data = "hello!";

    get data(): typeof this.#data {
        return this.#data;
    }

    set data(value: typeof this.#data) {
        this.#data = value;
    }
}

@magic-akari magic-akari marked this pull request as ready for review April 11, 2022 15:23
@evanw evanw merged commit 7b4b5e3 into evanw:master Apr 11, 2022
@jakebailey
Copy link

We're reverting the linked change in TS, so you probably want to revert it too.

@evanw
Copy link
Owner

evanw commented May 7, 2022

I think it should be ok to leave it in. The only purpose of parsing these types is to strip them, not to validate them. It's harmless if esbuild strips types that TypeScript considers a syntax error. Developers will just see the error from TypeScript and fix their code. The only harm would come if esbuild's type parser somehow behaved differently than TypeScript's type parser regarding which text is considered to be part of the type, which could happen with different ASI rules. But I don't believe that can happen here. It also sounds like this feature may be added back in the future in which case esbuild wouldn't need any further updates to support it.

That said, this case is probably a good reason for esbuild to ignore PRs for features in beta versions of TypeScript in the future.

@jakebailey
Copy link

That's true; I was just bulk notifying anyone who mentioned the PR. esbuild definitely wouldn't care about stuff after the colon for emit's sake.

Breaks like this should be rare, we just didn't notice it thanks to an inconvenient type cast.

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