Skip to content

Fix Windows compatibility#358

Merged
sindresorhus merged 3 commits intoimagemin:mainfrom
VitalyKrenel:master
Aug 11, 2021
Merged

Fix Windows compatibility#358
sindresorhus merged 3 commits intoimagemin:mainfrom
VitalyKrenel:master

Conversation

@VitalyKrenel
Copy link
Copy Markdown
Contributor

@VitalyKrenel VitalyKrenel commented May 23, 2020

PR adds explicit conversion for input file paths to Unix format which uses forward slash.

imagemin relies on globby package to calculate all file paths, when glob option is enabled. That package expects paths to use forward slashes as a path delimiter. When Windows path delimiter was encountered in the file name, the file was ignored, which resulted in imagemin not processing those files.

I also added a test for Windows delimiter, to make sure imagemin works with them correctly further down the road, but the test seems to me not super accurate because the tested path (generated by tempy) on Unix will contain both Unix and Windows path delimiters.

If you have any idea how to test the issue more specifically, feel free to update!

Fixes #352

Vitaly Krenel added 2 commits May 23, 2020 15:36
…min cross platform

imagemin relies on globby package which (at least as of time of this commit)
expects paths to use forward slash (Unix) and not backward slash (Windows).

To fix this issue, I've added additional step, converting filePaths to unix format.

Unix format (forward slash) delimiter is supported in Windows for the most of cases.
@see https://superuser.com/questions/176388/why-does-windows-use-backslashes-for-paths-and-unix-forward-slashes/176395\#176395
@150148313
Copy link
Copy Markdown

Is it solved

@150148313
Copy link
Copy Markdown

I set glob = false, all right, But I have to iterate all files

Base automatically changed from master to main January 25, 2021 21:10
@sindresorhus sindresorhus changed the title Fix imagemin not working on Windows because of the OS file path delimiter not supported by one of the dependecies Fix Windows compatibility Aug 11, 2021
@sindresorhus sindresorhus merged commit 7993551 into imagemin:main Aug 11, 2021
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.

Does't work on windows

3 participants