-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Remove ts.{Map,Set,ESMap,Iterator} and associated types #51439
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
Conversation
Checking sourcegraph, I think we may be safe to remove it wholesale. The only apparent match is: https://github.com/aspect-build/rules_ts/blob/fe974b9255a1e777f62c157dfa07fc363c350225/ts/private/ts_project_worker.js#L707 |
@typescript-bot perf test this |
Heya @jakebailey, I've started to run the perf test suite on this PR at 3255f20. You can monitor the build here. Update: The results are in! |
@jakebailey Here they are:
CompilerComparison Report - main..51439
System
Hosts
Scenarios
TSServerComparison Report - main..51439
System
Hosts
Scenarios
StartupComparison Report - main..51439
System
Hosts
Scenarios
Developer Information: |
In #50049, I removed our shims for
ts.Map
andts.Set
, requiring thatMap
andSet
be provided (either natively or by polyfill). Nobody complained.In a recent design meeting, we seemed to agree that it'd be a good idea to drop all of our own Map/Set types for 5.0 (#49332 (comment)). This PR removes them.
This is good post-modules because we don't have to import these commonly used types.
Potentially I can restore the values (and only drop the types) if we are worried about people having written
new ts.Map
ornew ts.Set
. I'm not sure how worthwhile that is.(This is technically the opposite of what we planned to do per the comments in the deprecated compat declarations, but we seemed to think it was fine during the meeting.)