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

Suggestions to avoid passing excessive arguments and use well-constructed options #51

Open
seahindeniz opened this issue Nov 16, 2020 · 1 comment

Comments

@seahindeniz
Copy link

seahindeniz commented Nov 16, 2020

Hi @semiromid

I have some suggestions about the types of parameters and their usages.

Quoting from README.md

const compress_images = require("compress-images");

function MyFun() {
  compress_images(
    "src/img/**/*.{jpg,JPG,jpeg,JPEG,png,svg,gif}",
    "build/img/",
    { compress_force: false, statistic: true, autoupdate: true },
    false,
    { jpg: { engine: "mozjpeg", command: ["-quality", "60"] } },
    { png: { engine: "pngquant", command: ["--quality=20-50", "-o"] } },
    { svg: { engine: "svgo", command: "--multipass" } },
    {
      gif: { engine: "gifsicle", command: ["--colors", "64", "--use-col=web"] },
    },
    function (err, completed) {
      if (completed === true) {
        // Doing something.
      }
    }
  );
}

Don't get me wrong but passing arguments that may don't even do with the input files, it's just kind of confusing and doesn't seem like it's well constructed.


What do you think about changing the structure to have something like this instead?

PS: Suggestions are written in TypeScript to have a better understanding of the properties and their types.
You can review them on the TS Playground

type PropsType = {
  output?: string;
  compress_force?: boolean;
  statistic?: boolean;
  autoupdate?: boolean;
  /** @default "./log/compress-images" */
  pathLog?: string;
  globoption?: boolean;
  compressors: {
    jpg?:
      | {
          engine: "jpegtran";
          /**
           * @example ['-trim', '-progressive', '-copy', 'none', '-optimize']
           * @see https://libjpeg-turbo.org/
           */
          command?: string[];
        }
      | {
          engine: "mozjpeg";
          /**
           * @example ['-quality', '10']
           * @see https://github.com/mozilla/mozjpeg/
           */
          command?: string[];
        }
      | {
          engine: "webp";
          /**
           * @example ['-q', '60']
           * @see https://developers.google.com/speed/webp/
           */
          command?: string[];
        }
      | {
          engine: "guetzli";
          /**
           * @example ['--quality', '84']
           * @see https://github.com/google/guetzli/
           * @description (Very long compresses on Win 8.1 https://github.com/google/guetzli/issues/238)
           */
          command?: string[];
        }
      | {
          engine: "jpegRecompress";
          /**
           * @example ['--quality', 'high', '--min', '60']
           * @see https://github.com/danielgtaylor/jpeg-archive/
           */
          command?: string[];
        }
      | {
          engine: "jpegoptim";
          /**
           * @example ['--all-progressive', '-d']
           * @see https://github.com/imagemin/jpegoptim-bin
           * @description To use jpegoptim you must npm install jpegoptim-bin --save, this library does not work properly on some OS and platforms. from https://github.com/imagemin/jpegoptim-bin Issues! May be a problems with installation and use on Win 7 x32 and maybe other OS: compress-images - issues/21 Caution! if do not specify '-d' all images will be compressed in the source folder and will be replaced. For Windows x32 and x63 also, you can use https://github.com/vikas5914/jpegoptim-win. Copy jpegoptim-32.exe and replace and rename in "node_modules\jpegoptim-bin\vendor\jpegoptim.exe"
           */
          command?: string[];
        }
      | {
          engine: "tinify";
          /**
           * @example ['copyright', 'creation', 'location']
           * @see https://tinypng.com/developers/reference/nodejs/
           */
          command?: string[];
          /**
           * @example "sefdfdcv335fxgfe3qw"
           */
          key?: string;
        };
    png?:
      | {
          engine: "pngquant";
          /**
           * @example ['--quality=20-50', '-o']
           * @see https://pngquant.org/
           * @description If you want to compress in the same folder, as example: ['--quality=20-50', '--ext=.png', '--force']. To use this library you need to install it manually. It does not work properly on some OS (Win 7 x32 and maybe other). npm install pngquant-bin --save Quality should be in format min-max where min and max are numbers in range 0-100. Can be problems with cyrillic filename issues/317
           */
          command?: string[];
        }
      | {
          engine: "optipng";
          /**
           * @see http://optipng.sourceforge.net/
           */
          command?: string[];
        }
      | {
          engine: "pngout";
          /**
           * @see http://advsys.net/ken/util/pngout.htm
           */
          command?: string[];
        }
      | {
          engine: "webp";
          /**
           * @example ['-q', '60']
           * @see https://developers.google.com/speed/webp/
           */
          command?: string[];
        }
      | {
          engine: "pngcrush";
          /**
           * @example ['-reduce', '-brute']
           * @see https://pmt.sourceforge.io/pngcrush/
           * @description (It does not work properly on some OS)
           */
          command?: string[];
        }
      | {
          engine: "tinify";
          /**
           * @example ['copyright', 'creation', 'location']
           * @see https://tinypng.com/developers/reference/nodejs/
           */
          command?: string[];
          /**
           * @example "sefdfdcv335fxgfe3qw"
           */
          key?: string;
        };
    gif?:
      | {
          engine: "gifsicle";
          /**
           * @example ['--colors', '64', '--use-col=web'] or ['--optimize']
           * @see http://www.lcdf.org/gifsicle/
           * @description To use this library you need to install it manually. It does not work properly on some OS. npm install gifsicle --save
           */
          command?: string[];
        }
      | {
          engine: "giflossy";
          /**
           * @example ['--lossy=80']
           * @see http://www.lcdf.org/gifsicle/
           * @description For Linux x64 and Mac OS X
           */
          command?: string[];
        }
      | {
          engine: "";
          /**
           * @example ['-f', '80', '-mixed', '-q', '30', '-m', '2']
           * @see https://developers.google.com/speed/webp/docs/gif2webp
           */
          command?: string[];
        };
    svg?: {
      engine: "svgo";
      /**
       * @example '--multipass'
       * @see https://www.npmjs.com/package/svgo/
       */
      command?: string;
    };
  };
};

// I can't find their types on the README.md so they are optional and have "any"
type StatisticType = {
  input?: any;
  path_out_new?: any;
  algorithm?: any;
  size_in?: any;
  size_output?: any;
  percent?: any;
  err?: any;
};

type CallbackType = (
  error: Error | null,
  status: boolean,
  statistic: StatisticType,
) => void;

type PromiseReturnType = {
  status: boolean;
  statistic: StatisticType;
};

function compressImages(
  path: string,
  props: PropsType,
): Promise<PromiseReturnType>;
function compressImages(
  path: string,
  props: PropsType,
  callback: CallbackType,
): void;
function compressImages(
  path: string,
  props: PropsType,
  callback?: CallbackType,
): void | Promise<PromiseReturnType> {
  console.log(path);
  console.log(props);

  if (!callback) {
    return Promise.resolve({
      status: true,
      statistic: {},
    });
  }

  callback(Error(), true, {});

  return undefined;
}

(async () => {
  // Example usage with callback
  compressImages(
    "src/img/**/*.{jpg,JPG,jpeg,JPEG,png,svg,gif}",
    {
      output: "build/img/",
      compress_force: false,
      statistic: true,
      autoupdate: true,
      compressors: {
        jpg: { engine: "mozjpeg", command: ["-quality", "60"] },
        png: { engine: "pngquant", command: ["--quality=20-50", "-o"] },
        svg: { engine: "svgo", command: "--multipass" },
        gif: {
          engine: "gifsicle",
          command: ["--colors", "64", "--use-col=web"],
        },
      },
    },
    (error, status, statistic) => {
      console.log(error);
      console.log(status);
      console.log(statistic);
    },
  );

  // Example usage with Promise
  try {
    const { statistic, status } = await compressImages(
      "src/img/**/*.{jpg,JPG,jpeg,JPEG,png,svg,gif}",
      {
        output: "build/img/",
        compress_force: false,
        statistic: true,
        autoupdate: true,
        compressors: {
          jpg: { engine: "mozjpeg", command: ["-quality", "60"] },
          png: { engine: "pngquant", command: ["--quality=20-50", "-o"] },
          svg: { engine: "svgo", command: "--multipass" },
          gif: {
            engine: "gifsicle",
            command: ["--colors", "64", "--use-col=web"],
          },
        },
      },
    );

    console.log(status);
    console.log(statistic);
  } catch (error) {
    console.error(error);
  }
})();
@Yuriy-Svetlov
Copy link
Owner

Hi @seahindeniz

Thank you for your suggestions!

// I can't find their types on the README.md so they are optional and have "any"
type StatisticType = {
input?: any;
path_out_new?: any;
algorithm?: any;
size_in?: any;
size_output?: any;
percent?: any;
err?: any;
};

Types located in the README.md API

input (type:string): Path to source image or images;

Your proposal is interesting. But I don't know what to say. So far I do not see the need for this, but of course, this does not mean that there is no such need. Therefore, I would like to know the opinion of others.

Thanks anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants