output paramater is ignored, original images are overwritten

[currentrame]

imagemin v5.0.0
imagemin-pngquant v5.0.0

code:
const imagemin = require('imagemin'); var pngComp = require("imagemin-pngquant"); imagemin(['/Users/m/projects/m/_test/tmp/in/*.{jpg,png}'], '/Users/m/projects/m/_test/tmp/out', { use: [ pngComp({quality: '65-80'}) ] }).then(files => { console.log(files); });

[kevva]

If you log files, what does it output?

[QuentinFchx]

https://github.com/imagemin/imagemin/blob/master/index.js#L11
const dest = output ? path.resolve(output, input) : null;
Not sure how this can work if input is already a relative path to the file ?
It should be the "matching" part of the glob

[kevva]

@QuentinFchx, happy if you could add a failing test.

[QuentinFchx]

test('should output at the specified location', async t => {
    const files1 = await m(['fixture.jpg'], 'output', {use: imageminJpegtran()});
    const files2 = await m(['output/*.jpg'], 'output', {use: imageminJpegtran()});
    t.is(path.relative(__dirname, files1[0].path), 'output/fixture.jpg');
    t.is(path.relative(__dirname, files2[0].path), 'output/fixture.jpg'); // will fail
});

[ldanet]

I'm having the same issue.
As QuentinFchx pointed out, there is a problem with globs, but there is another problem. currentrame and I are using absolute paths, and path.resolve(output, input) will simply return input if it is an absolute path. As a result, output gets completely ignored.
My app's working directory, input path and output path are completely unrelated, so relative paths don't make sense in my case.

[QuentinFchx]

We might want to use https://github.com/contra/glob2base.

[kevva]

Will try it and see what I come up with.

[kevva]

@QuentinFchx @currentrame @ldanet, I think #175 should fix it.

[QuentinFchx]

@kevva It works for simple patterns, but when you're using globstars (**/*.jpg) it does not

[kevva]

Isn't this exactly what you wrote earlier?

[QuentinFchx]

I wrote a specific test case that is fixed by your commit, however I forgot to mention this case can be extended.

Let's take the following use case:

I want to processbar.jpg located at images/foo/bar.jpg. I want the output to be send into the ouput folder with the same directory structure output/foo/bar.jpg.
According to the current implementation, and if I'm not mistaken:

• I call imagemin(['images/**/*.jpg'], 'output')
• The globby lib parse the globs and returns images paths (here images/foo/bar.jpg)
• The handleFile function is called with `handleFile('images/foo/bar.jpg', 'output')
• The file is output at output/bar.jpg instead of output/foo/bar.jpg

TLDR; The issue is that we lose the reference of the "matching" part of the glob. For each file, we want to separate the "static" part of the glob (/images) from the "dynamic" part /**/*.jpg, and append that "dynamic" part to the "output" (/output) one if provided.

Prior to 5.0.0 version, we were using vinyl-fs that used to keep that "matching pattern". It does not seem possible to achieve with globby alone.

[kevva]

Do we want that as the default behavior or behind an option? We only need to do this (I think) in any case https://github.com/sindresorhus/cpy/blob/master/index.js#L36, and maybe remove the matching parts.

[davidchase]

I think my path-union does the trick:

from index.js

- const dest = output ? path.join(output, path.basename(input)) : null;
+ const dest = output ? union(path.resolve(output), path.resolve(input)) : null;

running imagemin(['images/**/*.jpg'], 'output')

λ find images
images
images/fixture-corrupt.jpg
images/test
images/test/fixture.jpg

λ find output
output
output/images
output/images/fixture-corrupt.jpg
output/images/test
output/images/test/fixture.jpg