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

refactor: simplify hosts to directly assign tsModule.sys where possible #349

Merged
merged 1 commit into from
Jun 7, 2022
Merged

refactor: simplify hosts to directly assign tsModule.sys where possible #349

merged 1 commit into from
Jun 7, 2022

Commits on Jun 6, 2022

  1. refactor: simplify hosts to directly assign tsModule.sys where possible 

    - no need to duplicate types this way, which can and have changed over
  time -- it's always the same typings this way

- also reorganize `host.ts` to have similar categories of functions near
  each other, instead of a mix of functions wherever
  - similar to how I organized the tests for `host` as well
- shrink the code a bit this way too

- add a comment about `getDefaultLibFileName`'s confusing naming
  pointing to the TS issues about how this is an old mistake but
  changing it now would be breaking

- this is also how the TS Wiki recommends setting up hosts: https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API#incremental-build-support-using-the-language-services
  - NOTE: because of how `tsproxy` works to support alternate TS
    implementations, this does require that `tsModule` _exists_ at the
    time of instantiation, i.e. that `setTypescriptModule` has already
    been called
    - for `host.ts`, `LanguageServiceHost` is only instantiated after
      `setTypescriptModule`, but for `diagnostics-format-host.ts`, it is
      immediately instantiated (at the bottom of the file), hence why
      `getCurrentDirectory` can't just be assigned to `tsModule.sys`
      - there is a way to fix this, but the refactoring is more complex
        as it would require creating in `index.ts` and then passing it
        as an argument -- would want to refactor more at that point too,
        so leaving that out for now in this otherwise small, isolated
        refactor
  - for a different, but related reason, the `host.trace` tests have to
    mock `console` instead of just `console.log`, since `trace` would
    just be set to the old, unmocked `console.log` otherwise
    - as it's assigned directly to `console.log` now
    @agilgur5
    agilgur5 committed Jun 6, 2022
    Configuration menu
    Copy the full SHA
    40f44e8 View commit details
    Browse the repository at this point in the history