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

Normalise windows paths #7

Merged
merged 2 commits into from
Jul 21, 2022
Merged

Normalise windows paths #7

merged 2 commits into from
Jul 21, 2022

Conversation

timacdonald
Copy link
Contributor

@timacdonald timacdonald commented Jul 21, 2022

(hopefully) Fixes #6

My (possibly flawed) understanding of the problem is that the path.resolve function is converting POSIX paths to Windows paths on Windows machines.

So specifying...

FullReload(['path/to/files/**'])

results in...

["path\\to\\files\\**"]

after calling...

const files = Array.from(paths).map(path => resolve(root, path)).map(normalizePath)

chokidar.watch is then expecting to receive POSIX paths. From the docs...

Screen Shot 2022-07-21 at 1 23 12 pm

So it all breaks down.

This PR proposes that we pipe all paths through Vite's normalizePath function before passing to chokidar. I believe this is the recommended way of doing things:

Screen Shot 2022-07-21 at 1 06 53 pm

The normalizePath function is available in Vite 2 and Vite 3.

Warning: I do not have a windows machine to verify the bug, but also to verify this fix. This solutions needs some real-world testing to be done. I'm just taking a stab in the dark at the fix here 🤠

@timacdonald timacdonald changed the title Normalise paths Normalise windows paths Jul 21, 2022
@ElMassimo
Copy link
Owner

Thanks Tim! We'll need to add vite as a peer dependency now, I'll do that on a separate PR.

@ElMassimo ElMassimo merged commit cc47178 into ElMassimo:main Jul 21, 2022
@timacdonald
Copy link
Contributor Author

timacdonald commented Jul 21, 2022

@ElMassimo were you able to test this fix to validate it? I finally got a Windows virtual machine setup so I can test it if you didn't yourself?

edit: oh, I see that @guasam tested it for us! Thank you!

@timacdonald timacdonald deleted the windows-paths branch July 21, 2022 23:06
@timacdonald
Copy link
Contributor Author

p.s. my bad on the peer dependency! thanks for adding that in.

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.

Full Reload not working in Windows using \\ paths separator
2 participants