Skip to content

fix: error thrown from changeFileExtension when destination is undefined#419

Merged
sindresorhus merged 1 commit intoimagemin:mainfrom
kenrick95:fix/undefined-destination
Mar 7, 2025
Merged

fix: error thrown from changeFileExtension when destination is undefined#419
sindresorhus merged 1 commit intoimagemin:mainfrom
kenrick95:fix/undefined-destination

Conversation

@kenrick95
Copy link
Copy Markdown
Contributor

@kenrick95 kenrick95 commented Jan 31, 2025 •
edited
Loading

When destination is undefined or empty string, instead of:

If no destination is specified, no files will be written

imagemin throws error like below:

node:internal/process/promises:391
    triggerUncaughtException(err, true /* fromPromise */);
    ^

TypeError: Expected `filePath` to be a string, got `undefined`.
    at changeFileExtension (file:///REDACTED/node_modules/.pnpm/change-file-extension@0.1.1/node_modules/change-file-extension/index.js:5:9)
    at handleFile (file:///REDACTED/node_modules/.pnpm/imagemin@9.0.0/node_modules/imagemin/index.js:21:37)
    at async file:///REDACTED/node_modules/.pnpm/imagemin@9.0.0/node_modules/imagemin/index.js:54:13

Suspect commmit: 0f2a0aa#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346R6

This is because change-file-extension will throw error when filePath is undefined, unlike replace-ext which will just return without error thrown.

Fix here is to call change-file-extension when destination is truthy

@kenrick95 kenrick95 force-pushed the fix/undefined-destination branch from dd2eb8e to f47e00b Compare January 31, 2025 03:45
@kenrick95
Copy link
Copy Markdown
Contributor Author

kenrick95 commented Mar 7, 2025

Apologies for the ping. @sindresorhus would you help to review this PR? Thanks~

@sindresorhus sindresorhus merged commit 4094594 into imagemin:main Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kenrick95 @sindresorhus