Skip to content

Move most harness globals into namespaces #35530

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

Merged

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Dec 5, 2019

This way our transformation into modules doesn't need to attempt to preserve so many globals that we don't actually need as globals (since that preservation is really ugly).

Also now in this PR:

  • mapShim no longer relies on forward declarations of Map and indeterminate resolution of Iterator (neither fo these strategies work well with modules - it now has structural copies of both). Considering this removes a @ts-ignore, this seems like an improvement.
  • Renaming a bunch of files so their filenames doesn't clash with their namespace name (as we'd like to use their namespace name for a new file....)
  • Adding new files into testRunner to stub the namespaces from harness - surprisingly, this is the only layer where we use a namespace from another layer without redeclaring that namespace somewhere. Rather than creating special logic to handle this, adding a reference is easier.
  • Also, we have two namespaces in the harness - Utils and utils. This was problematic, as far as new filenames go. Now there's just Utils.
  • A couple more object shorthand references to namespace variables in the harness/runner are now explicit in their reference to a namespace.
  • Some globalish lets in the harness/runner now have a setter (as a let cannot be directly reassigned cross-module)
  • Lastly, a "breaking" public API change: ts.disableIncrementalParsing is no more. It was always false in our own codebase (since Enable incremental parsing by default. #1568 - 5 years ago), and we had no tests where it was changed to true. Doing a codesearch on github seems to indicate it's not actually a used public API, so removing it seems fine. In a way, this is a fix for Compiler API - Make disableIncrementalParsing not a global variable #35158. This is done because, well, you can't present a mutable, assignable member like this, as a module. If the functionality was needed, it could be exposed through a setter, but in any case, the variable would be no more.

@weswigham weswigham merged commit a78342a into microsoft:master Dec 6, 2019
@weswigham weswigham deleted the move-harness-globals-to-namespaces branch December 6, 2019 23:20
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.

2 participants