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

Core: Fix --static-dir with absolute path on Windows #13344

Merged
merged 56 commits into from
Dec 7, 2020

Conversation

ghengeveld
Copy link
Member

@ghengeveld ghengeveld commented Nov 30, 2020

Issue: #13336

What I did

Primarily this fixes the handling of the -s (--static-dir) flag for Windows users. This flag accepts a string in the shape of <dirname>:<endpoint>. Currently it fails to handle absolute paths on Windows: for a value like C:\path it would assume C to be the dirname and \path the endpoint. That bug is addressed here.

Besides fixing the bug at hand, I've extracted this piece of logic into a util, because it previously existed in two separate places. The util is now under unit test and the surrounding code has been cleaned up with some nicer logging. I've also replaced shelljs with copy from fs-extra. One less dependency 📉

How to test

  • Is this testable with Jest or Chromatic screenshots? No
  • Does this need a new example in the kitchen sink apps? No
  • Does this need an update to the documentation? No

If your answer is yes to any of these, please make sure to include it in your PR.

@ghengeveld ghengeveld requested a review from shilman November 30, 2020 22:19
@ghengeveld ghengeveld changed the title Extract static file handling to utils and make sure we handle Windows well. Core: Fix --static-dir with absolute path on Windows Nov 30, 2020
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

small thing: .test.js => .test.ts

@ghengeveld
Copy link
Member Author

small thing: .test.js => .test.ts

Done

@ghengeveld ghengeveld requested review from shilman and removed request for igor-dv, z4o4z, usulpro, CodeByAlex and thomasbertet December 7, 2020 14:05
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

LGTM - nice tests 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.