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

Added new config option convertAllTo … #23

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

netolicak
Copy link

for converting all image files to another format - can close issue #20

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation
  • Other

Description

Added new config option convertAllTo for converting all image files to another format - can close issue #20

Checks

  • PR adheres to the code style of this project
  • Related issues linked using fixes #number
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Lint and build have passed locally by running pnpm lint && pnpm build
  • Code changes have been verified in local
  • Documentation added/updated if necessary

Additional Context

@FatehAK
Copy link
Owner

FatehAK commented Jul 21, 2023

Hey @netolicak, thanks for the PR! Sorry I couldn't get back earlier. Could you please check the above lint issues? Also, I tried running your changes on my local, and they seem to be failing at:

bundler[filePath + "." + format].source = content;
                                                      ^
TypeError: Cannot set properties of undefined (setting 'source')

Since we are generating a new file and not altering an existing one, we should write a new file using fs at the same path as the existing file, I believe.

Also, here are a few suggestions for your PR:

  1. It can accept an array or a single string (i.e., convertAllTo: 'webp' or convertAllTo: ['webp', 'png']).

  2. If the extension of the file being converted is the same as the one present in convertAllTo, then there is no point in converting the file. We have to handle this scenario.

  3. Add TypeScript type completions for convertAllTo so that users are aware of which types to convert to in their IDE.

  4. If users pass an unknown file type that does not exist in options[extName], then we should probably throw an error.

  5. Add an output log indicating that file conversion to the formats specified in convertAllTo has been completed successfully in the logStats method.

@FatehAK FatehAK mentioned this pull request Jul 21, 2023
2 tasks
@Lastofthefirst
Copy link

This is an important feature. Is there something else in the vite plugin ecosystem that can make up for this feature in the meantime? @FatehAK

@FatehAK
Copy link
Owner

FatehAK commented Aug 16, 2023

A quick search gave me this @Lastofthefirst https://www.npmjs.com/package/vite-imagetools

@slashpot
Copy link

hi @FatehAK , is this request is going to be merged? thanks.

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.

4 participants