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

No errors on apparent type of bigint or symbol, even for recent targets #49104

Merged
merged 4 commits into from May 16, 2022

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented May 13, 2022

BigInt isn't resolved whenever lib < es2020, but it's not an error when target < es2020. I have a few ideas for improving this situation but for the RC I'm going to remove bigint from Object.freeze's signature.

Update: In discussion by the team, we decided that the checker should never error when a primitive like bigint or symbol can't resolve its associated apparent type. I removed the error message and restored bigint to Object.freeze in es5.d.ts.

Fixes #49101

`BigInt` isn't resolved whenever `lib < es2020`, but it's not an error
when `target < es2020`. I have a few ideas for improving this situation
but for the RC I'm going to remove `bigint` from Object.freeze's
signature.
@sandersn sandersn requested a review from DanielRosenwasser May 13, 2022
@sandersn sandersn requested a review from RyanCavanaugh May 13, 2022
@sandersn sandersn requested a review from weswigham May 13, 2022
@typescript-bot typescript-bot added the For Milestone Bug label May 13, 2022
@sandersn sandersn changed the title Remove bigint from Object.freeze in es5.d.ts No errors on apparent type of bigint or symbol, even for recent targets May 13, 2022
@sandersn
Copy link
Member Author

sandersn commented May 13, 2022

In discussion by the team, we decided that the checker should never error when a primitive like bigint or symbol can't resolve its associated apparent type. I removed the error message and restored bigint to Object.freeze in es5.d.ts.

Copy link
Member

@DanielRosenwasser DanielRosenwasser left a comment

The original comment is out of date, but given your last comment

In discussion by the team, we decided that the checker should never error when a primitive like bigint or symbol can't resolve its associated apparent type. I removed the error message and restored bigint to Object.freeze in es5.d.ts.

I think this is the right call. Referencing BigInt directly probably shouldn't work; however, trying to resolve the apparent type to BigInt for bigint (or to any new wrapper type of any new primitive) is something that should have a reasonable fallback behavior; nobody should get an error for that. Furthermore, the apparent type should be {} so that it is effectively opaque (other than the fact that it's "not null/undefined).

That seems to be the behavior of this PR, so 👍🏻

@sandersn sandersn merged commit 95731f0 into main May 16, 2022
11 checks passed
@sandersn sandersn deleted the remove-bigint-from-es5.d.ts branch May 16, 2022
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented May 16, 2022

@typescript-bot cherry-pick this to release-4.7

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 16, 2022

Heya @DanielRosenwasser, I've started to run the task to cherry-pick this into release-4.7 on this PR at 86fc8db. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 16, 2022

Hey @DanielRosenwasser, I've opened #49135 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this issue May 16, 2022
Component commits:
4c6e802 Remove bigint from Object.freeze in es5.d.ts
`BigInt` isn't resolved whenever `lib < es2020`, but it's not an error
when `target < es2020`. I have a few ideas for improving this situation
but for the RC I'm going to remove `bigint` from Object.freeze's
signature.

6d5bf6a Update other baselines

e1958f7 No errors for missing apparent type of bigint,symbol for any target

86fc8db Update test text
DanielRosenwasser pushed a commit that referenced this issue May 16, 2022
Component commits:
4c6e802 Remove bigint from Object.freeze in es5.d.ts
`BigInt` isn't resolved whenever `lib < es2020`, but it's not an error
when `target < es2020`. I have a few ideas for improving this situation
but for the RC I'm going to remove `bigint` from Object.freeze's
signature.

6d5bf6a Update other baselines

e1958f7 No errors for missing apparent type of bigint,symbol for any target

86fc8db Update test text

Co-authored-by: Nathan Shively-Sanders <nathansa@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants