mapbox / appropriate-images Goto Github PK

I'm trying to process the attached image.

When I provide the following sizes...

heroSnmBaseMtFuji: {
    basename: 'hero-snm-base-mt-fuji.png',
    sizes: [
      { width: 600 },
      { width: 700 },
      { widht: 800 },
      { width: 900 },
      { width: 1200 },
      { width: 1600 },
      { width: 1400 },
      { width: 1800 }
    ]
  }

It doesn't create the 800 size. Here's the output:

Saved hero-snm-base-mt-fuji-1200.png
Saved hero-snm-base-mt-fuji-1200.webp
Saved hero-snm-base-mt-fuji-1400.png
Saved hero-snm-base-mt-fuji-1400.webp
Saved hero-snm-base-mt-fuji-1600.png
Saved hero-snm-base-mt-fuji-1600.webp
Saved hero-snm-base-mt-fuji-1800.png
Saved hero-snm-base-mt-fuji-1800.webp
Saved hero-snm-base-mt-fuji-600.png
Saved hero-snm-base-mt-fuji-600.webp
Saved hero-snm-base-mt-fuji-700.png
Saved hero-snm-base-mt-fuji-700.webp
Saved hero-snm-base-mt-fuji-900.png
Saved hero-snm-base-mt-fuji-900.webp
Saved hero-snm-base-mt-fuji-undefined.png
Saved hero-snm-base-mt-fuji-undefined.webp

We should probably make this a monorepo that includes https://github.com/mapbox/appropriate-images-get-url and https://github.com/mapbox/appropriate-images-react. Both of those packages should have a release any time this one does, so a monorepo will make things easier. It might also make the relationship between the packages easier to grasp for users.

@samanpwbb @tristen @mayagao, interested in your opinions on this one.

The idea I was going to port over from dotcom's MagicImage is that the React component measures its available width (https://github.com/mapbox/react-simple-surveyor) then picks the image to load based on the width and webp support.

Another possible approach: create a <picture> element and use https://github.com/scottjehl/picturefill for browser support.

Advantages of a <picture> element:

• In some sense it's a more simple approach, since the browser should do most of the heavy lifting.
• <picture> is standards-based instead of custom-code-based.
• Browser detects webp support on its own.
• Probably is more efficient in browsers that support <picture>.

Disadvantages:

• Picture selection must be done with media queries. So instead of making a choice based on the width of the container, we'll make the choice based on the width of the viewport. I'm coming to think that this might not be too much of a disadvantage, though, in real use-cases, where we are more than capable of coming up with a media query that fits the bill.

What do you think? Is the niceness of automatically measuring the available width, instead of having to write media queries, worth the extra code overhead? Or should we use <picture>?

The problem

When attempting to optimize XXX png files, we received the following error:

Error at makeError (/work/ios-sdk/node_modules/imagemin-pngquant/node_modules/execa/lib/error.js:59:11) at handlePromise (/work/ios-sdk/node_modules/imagemin-pngquant/node_modules/execa/index.js:114:26) at runMicrotasks () at processTicksAndRejections (internal/process/task_queues.js:97:5) at async /work/ios-sdk/node_modules/p-pipe/index.js:12:19 at async handleFile (/work/ios-sdk/node_modules/imagemin/index.js:21:9) at async /work/ios-sdk/node_modules/imagemin/index.js:54:13 at async Promise.all (index 11) at async Promise.all (index 0) at async module.exports (/work/ios-sdk/node_modules/@mapbox/appropriate-images/node_modules/p-finally/index.js:9:11)npm ERR! code ELIFECYCLEnpm ERR! errno 1npm ERR! [email protected] build-images: mkdir -p src/img/dist && node bin/build-image-config.js && node bin/appropriate-images.js --allnpm ERR! Exit status 1npm ERR!npm ERR! Failed at the [email protected] build-images script.npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

After some prodding, we were able to return a more useful error message: imagemin is running out of memory:

Error: Command failed with ENOMEM: /work/ios-sdk/node_modules/pngquant-bin/vendor/pngquant -
spawn ENOMEM
at ChildProcess.spawn (internal/child_process.js:407:11)
at Object.spawn (child_process.js:548:9)
at execa (/work/ios-sdk/node_modules/imagemin-pngquant/node_modules/execa/index.js:80:26)
at /work/ios-sdk/node_modules/imagemin-pngquant/index.js:57:21
at /work/ios-sdk/node_modules/p-pipe/index.js:12:25
at handleFile (/work/ios-sdk/node_modules/imagemin/index.js:21:54)
at async /work/ios-sdk/node_modules/imagemin/index.js:54:13
at async Promise.all (index 209)
at async module.exports (/work/ios-sdk/node_modules/@mapbox/appropriate-images/lib/optimize.js:47:34)
at async module.exports (/work/ios-sdk/node_modules/@mapbox/appropriate-images/node_modules/p-finally/index.js:9:11)

Proposed solution

In https://stackoverflow.com/a/63573743, the poster found success by using p-limit:

The solution for us, was to use plimit to limit the number of concurrent promises to 5 or 10. On the same number of images (20k+) the VIRT mem usage never rose, neither did RES, and we stopped getting ENOMEM errors.

I tested this by optimizing each file and and using p-limit and it worked.

Example:

    const makeItHappen = createTemporaryDirectory()
      .then(() => Promise.all(generateSizeVariants))
      .then((result) => {
        if (configErrors.length) return Promise.reject(configErrors);
        return result;
      })
      .then((result) => {
        const optimizeQueue = [];

        for (var res in result) {
          optimizeQueue.push(
            limit(() => optimize(_.flatten(result[res]), options))
          );
        }
        return Promise.all(optimizeQueue);
      });

Next steps

We should consider adding a max concurrency flag that, when enabled, will use p-limit to handle optimizing each file.

cc @colleenmcginnis

I made an incorrect config and an image file was not found. Here's the message I got:

(node:69793) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: Input file is missing or of an unsupported image format
(node:69793) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
Error: Input file is missing or of an unsupported image format
    at Error (native)

That is not helpful.

I suspect that it's showing what sharp throws when you pass it a bad path.

We should make this more user-friendly, pointing at the specific entry in the config that has the problem.

It might be nice to preserve the source directory structure. Right now you can specify a base path with subdirectories (e.g. basename: 'examples/offline/simple-offline.png'), but the results are flattened into a single directory.

It appears that specifying a width property for an image, which is the exact width of the source image, prevents the webp format from being generated.

Take a look at the script's output for these three images:

Saved difficulty-icon-advanced-88.png
Saved difficulty-icon-advanced-88.png
Saved difficulty-icon-intermediate-102.png
Saved difficulty-icon-intermediate-102.png
Saved difficulty-icon-intro-56.png
Saved difficulty-icon-intro-56.png

And after subtracting one pixel from each image's desired width:

Saved difficulty-icon-advanced-87.png
Saved difficulty-icon-advanced-87.webp
Saved difficulty-icon-intermediate-101.png
Saved difficulty-icon-intermediate-101.webp
Saved difficulty-icon-intro-55.png
Saved difficulty-icon-intro-55.webp

The website for sharp has moved to https://sharp.pixelplumbing.com/. The change happened sometime around July 2019.

I ran into an issue today installing appropriate-images. Based on the error, I believe the issue is stemming from sharp. See output of error below.

When I tried to npm install this repo, I saw the same error as when I tried to npm install the module. When I updated sharp, the error went away.

Also, based on this warning:

warning @mapbox/appropriate-images > imagemin-mozjpeg > mozjpeg > bin-build > download > gulp-decompress > [email protected]: gulp-util is deprecated - replace it, following the guidelines at https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5

It may be good to also bump imagemin-mozjpeg.

I can create a PR to bump the versions, but I wanted to get a gut check.

Output:

katydecorah at mapbox in ~/Documents/GitHub/mapbox/mapbox-gl-js on approp-image [!?]
$ yarn install
yarn install v1.13.0
(node:68013) ExperimentalWarning: The fs.promises API is experimental
[1/5] 🔍  Validating package.json...
[2/5] 🔍  Resolving packages...
warning @mapbox/appropriate-images > imagemin-mozjpeg > mozjpeg > bin-build > download > gulp-decompress > [email protected]: gulp-util is deprecated - replace it, following the guidelines at https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5
[3/5] 🚚  Fetching packages...
[4/5] 🔗  Linking dependencies...
warning " > [email protected]" has unmet peer dependency "webpack@^4.3.0".
warning " > [email protected]" has incorrect peer dependency "rollup@^0.56.2".
[5/5] 🔨  Building fresh packages...
[7/8] ⠠ fsevents
[-/8] ⠠ waiting...
[8/8] ⠠ gl
[4/8] ⠠ sharp
error /Users/katydecorah/Documents/GitHub/mapbox/mapbox-gl-js/node_modules/sharp: Command failed.
Exit code: 1
Command: node-gyp rebuild
Arguments: 
Directory: /Users/katydecorah/Documents/GitHub/mapbox/mapbox-gl-js/node_modules/sharp
Output:
gyp info it worked if it ends with ok
gyp info using [email protected]
gyp info using [email protected] | darwin | x64
gyp info spawn /usr/bin/python
gyp info spawn args [ '/Users/katydecorah/.nvm/versions/node/v10.1.0/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/katydecorah/Documents/GitHub/mapbox/mapbox-gl-js/node_modules/sharp/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/katydecorah/.nvm/versions/node/v10.1.0/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/katydecorah/.node-gyp/10.1.0/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/Users/katydecorah/.node-gyp/10.1.0',
gyp info spawn args   '-Dnode_gyp_dir=/Users/katydecorah/.nvm/versions/node/v10.1.0/lib/node_modules/npm/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/Users/katydecorah/.node-gyp/10.1.0/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/Users/katydecorah/Documents/GitHub/mapbox/mapbox-gl-js/node_modules/sharp',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.' ]
Ignoring commonmarker-0.17.12 because its extensions are not built.  Try: gem pristine commonmarker --version 0.17.12
Ignoring commonmarker-0.17.11 because its extensions are not built.  Try: gem pristine commonmarker --version 0.17.11
gyp: Call to 'which brew >/dev/null 2>&1 && eval $(brew --env) && echo $PKG_CONFIG_LIBDIR || true' returned exit status 0 while in binding.gyp. while trying to load binding.gyp
gyp ERR! configure error 
gyp ERR! stack Error: `gyp` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onCpExit (/Users/katydecorah/.nvm/versions/node/v10.1.0/lib/node_modules/npm/node_modules/node-gyp/lib/configure.js:345:16)
gyp ERR! stack     at ChildProcess.emit (events.js:182:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:235:12)
gyp ERR! System Darwin 18.2.0
gyp ERR! command "/Users/katydecorah/.nvm/versions/node/v10.1.0/bin/node" "/Users/katydecorah/.nvm/versions/node/v10.1.0/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /Users/katydecorah/Documents/GitHub/mapbox/mapbox-gl-js/node_modules/sharp

I'd like to be able to define a custom suffix eg [email protected].
And also for images with defined width and height I want the same notation as above.
So, there should be a suffix fn that has width and height arguments so I can build the suffix like I want.
Thx!

getAppropriateImagesUrl

• runs in the browser
• does not require a sharp install
• is a dependency of things like https://github.com/mapbox/appropriate-images-react that also don't need a sharp install

So I think it should be its own package. I'll extract it.

As the title says...