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.

Sign up for GitHub

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

Jump to bottom

Update default target from ES3 to ES5 #51731

Merged
merged 8 commits into from
Dec 9, 2022
Merged

Update default target from ES3 to ES5 #51731

merged 8 commits into from
Dec 9, 2022

Conversation

RyanCavanaugh
Copy link
Member

This staging work for the removal of ES3 as part of #51000. 99.9% of the changes are parked in one commit for reviewal ease.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Dec 2, 2022
@andrewbranch
Copy link
Member

Can the tests with source maps be updated instead of deleted? The verify.getEmitOutput failure should show the actual output, which can be pasted back into the test. It looks like those calls are just there to ensure the test setup is valid, and the subsequent calls are the ones that test the feature. Since source map stuff is a pain to test, I’d hate to give up any of what coverage we do have.

jakebailey
jakebailey reviewed Dec 8, 2022
View reviewed changes
tests/baselines/reference/ES3For-ofTypeCheck1.errors.txt
@@ -1,7 +0,0 @@
tests/cases/conformance/statements/for-ofStatements/ES3For-ofTypeCheck1.ts(1,15): error TS2494: Using a string in a 'for...of' statement is only supported in ECMAScript 5 and higher.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these "ES3For" tests be modified to still test ES3, but explicitly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess maybe it doesn't matter if the next PR is "delete ES3"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do a post-pass to clean up any irrelevant tests

@RyanCavanaugh
Copy link
Member Author

Deleted tests are back with latest commit

@RyanCavanaugh RyanCavanaugh merged commit a3dbe7d into microsoft:main Dec 9, 2022
@RyanCavanaugh RyanCavanaugh deleted the remove-es3-test-target branch December 9, 2022 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakebailey jakebailey jakebailey left review comments

@andrewbranch andrewbranch andrewbranch approved these changes

@sheetalkamat sheetalkamat sheetalkamat approved these changes

Assignees

@RyanCavanaugh RyanCavanaugh

Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@RyanCavanaugh @andrewbranch @jakebailey @sheetalkamat @typescript-bot