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

fix: don't duplicate type-only check on Windows #385

Merged
merged 1 commit into from
Jul 21, 2022
Merged

fix: don't duplicate type-only check on Windows #385

merged 1 commit into from
Jul 21, 2022

Conversation

agilgur5
Copy link
Collaborator

@agilgur5 agilgur5 commented Jul 20, 2022 •
edited
Loading

Summary

Ensure that the type-only / "missed" type-check doesn't duplicate on Windows

Details

  • gotta remember to normalize file names 🙃

    • this fixes a failing test on Windows by fixing the underlying bug

  • refactor: improve normalization in errors.spec

    • normalize all calls to local, instead of just one
    • this doesn't affect the tests, only the source code change does, but I added this first and it's better practice / test future-proofing

Review Notes

I confirmed on CI on my fork that tests pass on Windows now

  • EDIT: and as double check, they pass here on this PR too

@agilgur5
fix: don't duplicate type-only check on Windows
7510c80 
- gotta remember to normalize file names 🙃
  - this fixes a failing test on Windows by fixing the underlying bug

- refactor: improve normalization in errors.spec
  - normalize _all_ calls to `local`, instead of just one
  - this doesn't affect the tests, only the source code change does, but I added this first and it's better practice / test future-proofing
@agilgur5 agilgur5 added kind: bug Something isn't working properly topic: OS separators Path normalization between OSes. Windows = '\', POSIX = '/'. Rollup resolve = native, TS = POSIX scope: tests Tests could be improved. Or changes that only affect tests labels Jul 20, 2022
@agilgur5 agilgur5 mentioned this pull request Jul 20, 2022
Merged
@agilgur5 agilgur5 added the topic: type-only / emit-less imports Related to importing type-only files that will not be emitted label Jul 20, 2022
This was referenced Jul 20, 2022
Merged
Merged
@ezolenko ezolenko merged commit c2b3db2 into ezolenko:master Jul 21, 2022
@agilgur5 agilgur5 deleted the fix-test-type-only-windows branch July 2, 2023 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
kind: bug Something isn't working properly scope: tests Tests could be improved. Or changes that only affect tests topic: OS separators Path normalization between OSes. Windows = '\', POSIX = '/'. Rollup resolve = native, TS = POSIX topic: type-only / emit-less imports Related to importing type-only files that will not be emitted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@agilgur5 @ezolenko