malchata / rollup-plugin-imagemin

Imagemin meets Rollup!
25 stars 7 forks source link

Fix incorrect outputFileName slash on Windows #12

Closed sxxov closed 3 years ago

sxxov commented 3 years ago

When building on Windows, there'll be an error in the browser when trying to import the generated module:

Uncaught TypeError: Failed to construct 'URL': Invalid URL

This is because path.join uses backslashes on Windows instead of forward ones that new URL() expects:

// error
new URL('foo\bar', import.meta.url);

// all good
new URL('foo\bar'.replace(/\\/g, '/'), import.meta.url);
sxxov commented 3 years ago

@malchata I am a better man since i wrote that PR.

The correct way to fix it would be to replace path.join with path.posix.join rather than do it with regex

malchata commented 3 years ago

I deeply apologize for the delay in merging this @Sxxov. I had some watch settings for repositories that were burying notifications for this repo and it slipped.

I published an update to this package you may use to test. To ensure I don't unintentionally break dependent applications through a normal install, I published a version containing these (and other) proposed changes to the package's next tag, which you can install as npm i rollup-plugin-imagemin@next. Once you (and other PR submitters) are satisfied, I will republish the package without the next tag.

Please also note that, due to a recently added warning in Rollup, the default export has been dropped and is now a single named export named imagemin. You can check the README for exact usage details.

malchata commented 3 years ago

Ack, you slipped in a comment while I was writing mine!

If you want to submit another PR to use that instead, I'd be happy to merge it. :)

sxxov commented 3 years ago

@malchata Hey no problem with the PR accepting things, shit happens

I'll test it out when I get the chance to, especially thanks for the rollup heads-up too!

malchata commented 3 years ago

No worries, and just a heads up: I changed the default branch from master to main, so you may need to update your local or re-clone.