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

refactor: split out a common typecheckFile func #344

Merged
merged 2 commits into from
Jun 20, 2022

Conversation

agilgur5
Copy link
Collaborator

@agilgur5 agilgur5 commented Jun 4, 2022

Summary

Split out a common typecheckFile function used in 3+ places to DRY up more code

Details

  • this is used in 3 places and going to be more for the code I'm adding to fix type-only imports in fix: type-check included files missed by transform (type-only files) #345 (and probably more type-only PRs in the future)

    • so DRY it up and standardize the functionality too
      • some places had noErrors = false in one place while others had it in another
      • same for printDiagnostics
      • but the ordering actually doesn't matter, so just keep it consistent and the same
        • and then can split a common function that does both out
  • technically, now getDiagnostics (from refactor: prefer native methods to lodash where possible #328) is now only used in typecheckFile, so I could combine the two together, but I'm refactoring that one up a little

    • but this a good, small example of how refactoring one part of a codebase can make it easier to identify more similar pieces and then refactor even more

- this is used in 3 places and going to be more for the code I'm adding
  to fix type-only imports
  - so DRY it up and standardize the functionality too
    - some places had `noErrors = false` in one place while others had
      it in another
    - same for `printDiagnostics`
    - but the ordering actually doesn't matter, so just keep it
      consistent and the same
      - and then can split a common function that does both out

- technically, now getDiagnostics is _only_ used in typecheckFile, so
  I could link to the two together, but I'm refactoring that one up
  a little
  - but this a good, small example of how refactoring one part of a
    codebase can make it easier to identify more similar pieces and then
    refactor even more
@agilgur5 agilgur5 added the kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs label Jun 4, 2022
@agilgur5
Copy link
Collaborator Author

@ezolenko this one's ready to merge -- not sure if you skipped it intentionally or not

@ezolenko
Copy link
Owner

@agilgur5 just the comment on line 204

@agilgur5
Copy link
Collaborator Author

agilgur5 commented Jun 17, 2022

@ezolenko oh, I don't see a comment -- I think maybe you didn't hit "Submit Review"? Code comments are saved as draft by default

src/index.ts Show resolved Hide resolved
@ezolenko
Copy link
Owner

Ah, no wonder...

@ezolenko ezolenko merged commit b9dce9d into ezolenko:master Jun 20, 2022
@agilgur5 agilgur5 added the topic: TS Compiler API Docs Related to the severely lacking TS Compiler API Docs label Jul 6, 2022
@agilgur5 agilgur5 deleted the refactor-split-typecheck-func branch July 2, 2023 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs topic: TS Compiler API Docs Related to the severely lacking TS Compiler API Docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants