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

feat(next/image)!: remove squoosh in favor of sharp as optional dep #61696

Merged
merged 10 commits into from
Feb 6, 2024

Conversation

styfle
Copy link
Member

@styfle styfle commented Feb 6, 2024

History

Previously, we added support for squoosh because it was a wasm implementation that "just worked" on all platforms when running next dev for the first time. However, it was slow so we always recommended manually installing sharp for production use cases running next build and next start.

Now that sharp supports webassembly, we no longer need to maintain squoosh, so it can be removed. We also don't need to make the user install sharp manually because it can be installed under optionalDependencies. I left it optional in case there was some platform that still needed to manually install the wasm variant with npm install --cpu=wasm32 sharp such as codesandbox/stackblitz (I don't believe sharp has any fallback built in yet).

Since we can guarantee sharp, we can also remove get-orientation dep and upgrade image-size dep.

I also moved an existing sharp test into its own fixture since it was unrelated to image optimization.

Related Issues

Copy link

socket-security bot commented Feb 6, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/image-size@1.1.1 filesystem +2 71.6 kB netroy
npm/sharp@0.33.2 environment Transitive: filesystem, shell +10 761 kB lovell

🚮 Removed packages: npm/@types/sharp@0.29.3, npm/get-orientation@1.1.2, npm/image-size@0.9.3, npm/image-size@1.0.0

View full report↗︎

Copy link

socket-security bot commented Feb 6, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

@ijjk
Copy link
Member

ijjk commented Feb 6, 2024

Tests Passed

@ijjk
Copy link
Member

ijjk commented Feb 6, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js remove-squoosh-add-sharp-optionaldep Change
buildDuration 11.6s 10.9s N/A
buildDurationCached 6.2s 5.4s N/A
nodeModulesSize 196 MB 318 MB ⚠️ +122 MB
nextStartRea..uration (ms) 433ms 430ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js remove-squoosh-add-sharp-optionaldep Change
3f784ff6-HASH.js gzip 53.4 kB 53.4 kB
423.HASH.js gzip 185 B 181 B N/A
68-HASH.js gzip 29.8 kB 29.8 kB N/A
framework-HASH.js gzip 45.2 kB 45.2 kB
main-app-HASH.js gzip 238 B 239 B N/A
main-HASH.js gzip 31.8 kB 31.8 kB N/A
webpack-HASH.js gzip 1.7 kB 1.7 kB
Overall change 100 kB 100 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js remove-squoosh-add-sharp-optionaldep Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary vercel/next.js remove-squoosh-add-sharp-optionaldep Change
_app-HASH.js gzip 194 B 195 B N/A
_error-HASH.js gzip 182 B 181 B N/A
amp-HASH.js gzip 502 B 501 B N/A
css-HASH.js gzip 320 B 322 B N/A
dynamic-HASH.js gzip 2.5 kB 2.5 kB N/A
edge-ssr-HASH.js gzip 255 B 256 B N/A
head-HASH.js gzip 350 B 349 B N/A
hooks-HASH.js gzip 368 B 369 B N/A
image-HASH.js gzip 4.19 kB 4.21 kB N/A
index-HASH.js gzip 257 B 256 B N/A
link-HASH.js gzip 2.61 kB 2.61 kB N/A
routerDirect..HASH.js gzip 310 B 311 B N/A
script-HASH.js gzip 384 B 383 B N/A
withRouter-HASH.js gzip 306 B 308 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 106 B 106 B
Client Build Manifests
vercel/next.js canary vercel/next.js remove-squoosh-add-sharp-optionaldep Change
_buildManifest.js gzip 483 B 484 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js remove-squoosh-add-sharp-optionaldep Change
index.html gzip 529 B 527 B N/A
link.html gzip 541 B 538 B N/A
withRouter.html gzip 524 B 522 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js remove-squoosh-add-sharp-optionaldep Change
edge-ssr.js gzip 94 kB 94.1 kB N/A
page.js gzip 149 kB 149 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js remove-squoosh-add-sharp-optionaldep Change
middleware-b..fest.js gzip 621 B 622 B N/A
middleware-r..fest.js gzip 151 B 149 B N/A
middleware.js gzip 37.7 kB 37.7 kB N/A
edge-runtime..pack.js gzip 1.92 kB 1.92 kB
Overall change 1.92 kB 1.92 kB
Next Runtimes
vercel/next.js canary vercel/next.js remove-squoosh-add-sharp-optionaldep Change
app-page-exp...dev.js gzip 166 kB 166 kB
app-page-exp..prod.js gzip 95.1 kB 95.1 kB
app-page-tur..prod.js gzip 96.9 kB 96.9 kB
app-page-tur..prod.js gzip 91.4 kB 91.4 kB
app-page.run...dev.js gzip 135 kB 135 kB
app-page.run..prod.js gzip 90 kB 90 kB
app-route-ex...dev.js gzip 22 kB 22 kB
app-route-ex..prod.js gzip 14.8 kB 14.8 kB
app-route-tu..prod.js gzip 14.8 kB 14.8 kB
app-route-tu..prod.js gzip 14.6 kB 14.6 kB
app-route.ru...dev.js gzip 21.7 kB 21.7 kB
app-route.ru..prod.js gzip 14.6 kB 14.6 kB
pages-api-tu..prod.js gzip 9.43 kB 9.43 kB
pages-api.ru...dev.js gzip 9.7 kB 9.7 kB
pages-api.ru..prod.js gzip 9.43 kB 9.43 kB
pages-turbo...prod.js gzip 22 kB 22 kB
pages.runtim...dev.js gzip 22.7 kB 22.7 kB
pages.runtim..prod.js gzip 22 kB 22 kB
server.runti..prod.js gzip 49.7 kB 49.7 kB
Overall change 922 kB 922 kB
Diff details
Diff for image-HASH.js
@@ -1,7 +1,7 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [358],
   {
-    /***/ 1552: /***/ function (
+    /***/ 4070: /***/ function (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/image",
         function () {
-          return __webpack_require__(2599);
+          return __webpack_require__(2369);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 9272: /***/ function (module, exports, __webpack_require__) {
+    /***/ 3299: /***/ function (module, exports, __webpack_require__) {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -40,15 +40,15 @@
         __webpack_require__(422)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(8286)
+        __webpack_require__(4830)
       );
-      const _getimgprops = __webpack_require__(985);
-      const _imageconfig = __webpack_require__(8749);
-      const _imageconfigcontextsharedruntime = __webpack_require__(7829);
-      const _warnonce = __webpack_require__(3652);
-      const _routercontextsharedruntime = __webpack_require__(6120);
+      const _getimgprops = __webpack_require__(3301);
+      const _imageconfig = __webpack_require__(2958);
+      const _imageconfigcontextsharedruntime = __webpack_require__(6869);
+      const _warnonce = __webpack_require__(2179);
+      const _routercontextsharedruntime = __webpack_require__(7554);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(5732)
+        __webpack_require__(9687)
       );
       // This is replaced by webpack define plugin
       const configEnv = {
@@ -373,7 +373,7 @@
       /***/
     },
 
-    /***/ 985: /***/ function (
+    /***/ 3301: /***/ function (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -389,9 +389,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(3652);
-      const _imageblursvg = __webpack_require__(1668);
-      const _imageconfig = __webpack_require__(8749);
+      const _warnonce = __webpack_require__(2179);
+      const _imageblursvg = __webpack_require__(892);
+      const _imageconfig = __webpack_require__(2958);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -760,7 +760,7 @@
       /***/
     },
 
-    /***/ 1668: /***/ function (__unused_webpack_module, exports) {
+    /***/ 892: /***/ function (__unused_webpack_module, exports) {
       "use strict";
       /**
        * A shared function, used on both client and server, to generate a SVG blur placeholder.
@@ -815,7 +815,7 @@
       /***/
     },
 
-    /***/ 9582: /***/ function (
+    /***/ 3612: /***/ function (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -842,10 +842,10 @@
         },
       });
       const _interop_require_default = __webpack_require__(2430);
-      const _getimgprops = __webpack_require__(985);
-      const _imagecomponent = __webpack_require__(9272);
+      const _getimgprops = __webpack_require__(3301);
+      const _imagecomponent = __webpack_require__(3299);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(5732)
+        __webpack_require__(9687)
       );
       function getImageProps(imgProps) {
         const { props } = (0, _getimgprops.getImgProps)(imgProps, {
@@ -877,7 +877,7 @@
       /***/
     },
 
-    /***/ 5732: /***/ function (__unused_webpack_module, exports) {
+    /***/ 9687: /***/ function (__unused_webpack_module, exports) {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -912,7 +912,7 @@
       /***/
     },
 
-    /***/ 2599: /***/ function (
+    /***/ 2369: /***/ function (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -933,15 +933,15 @@
 
       // EXTERNAL MODULE: ./node_modules/.pnpm/react@18.2.0/node_modules/react/jsx-runtime.js
       var jsx_runtime = __webpack_require__(1527);
-      // EXTERNAL MODULE: ./node_modules/.pnpm/file+..+main-repo+packages+next+next-packed.tgz_react-dom@18.2.0_react@18.2.0/node_modules/next/image.js
-      var next_image = __webpack_require__(1577);
+      // EXTERNAL MODULE: ./node_modules/.pnpm/file+..+diff-repo+packages+next+next-packed.tgz_react-dom@18.2.0_react@18.2.0/node_modules/next/image.js
+      var next_image = __webpack_require__(73);
       var image_default = /*#__PURE__*/ __webpack_require__.n(next_image); // CONCATENATED MODULE: ./pages/nextjs.png
       /* harmony default export */ var nextjs = {
         src: "/_next/static/media/nextjs.cae0b805.png",
         height: 1347,
         width: 1626,
         blurDataURL:
-          "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAHCAAAAAAQMlOCAAAAMElEQVR42hXJOQ4AIBDDwPz/q9BRcG6MMpUlCweWCSPqnrWxmK33Ucjeq16CyHKYD8vhNzo5Br4tAAAAAElFTkSuQmCC",
+          "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAHCAMAAAACh/xsAAAAD1BMVEX////x8fH6+vrb29vo6Oh8o70bAAAACXBIWXMAAAsTAAALEwEAmpwYAAAAH0lEQVR4nGNgwARMjIyMjCAGCzMzMwsTRISJCcRABwAEcAAkLCQfgAAAAABJRU5ErkJggg==",
         blurWidth: 8,
         blurHeight: 7,
       }; // CONCATENATED MODULE: ./pages/image.js
@@ -964,12 +964,12 @@
       /***/
     },
 
-    /***/ 1577: /***/ function (
+    /***/ 73: /***/ function (
       module,
       __unused_webpack_exports,
       __webpack_require__
     ) {
-      module.exports = __webpack_require__(9582);
+      module.exports = __webpack_require__(3612);
 
       /***/
     },
@@ -980,7 +980,7 @@
       return __webpack_require__((__webpack_require__.s = moduleId));
     };
     /******/ __webpack_require__.O(0, [888, 774, 179], function () {
-      return __webpack_exec__(1552);
+      return __webpack_exec__(4070);
     });
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;
Commit: 94c132d

@styfle styfle marked this pull request as ready for review February 6, 2024 15:23
@styfle styfle removed the request for review from a team February 6, 2024 15:23
Copy link

vercel-spaces bot commented Feb 6, 2024

Notifying the following users due to files changed in this PR based on this repo's notify modifiers:

@timneutkens, @ijjk, @shuding:

packages/next/src/server/lib/squoosh/.vercel.approvers
packages/next/src/server/lib/squoosh/LICENSE
packages/next/src/server/lib/squoosh/avif/avif_enc.d.ts
packages/next/src/server/lib/squoosh/avif/avif_node_dec.js
packages/next/src/server/lib/squoosh/avif/avif_node_dec.wasm
packages/next/src/server/lib/squoosh/avif/avif_node_enc.js
packages/next/src/server/lib/squoosh/avif/avif_node_enc.wasm
packages/next/src/server/lib/squoosh/codecs.ts
packages/next/src/server/lib/squoosh/emscripten-types.d.ts
packages/next/src/server/lib/squoosh/emscripten-utils.ts
packages/next/src/server/lib/squoosh/image_data.ts
packages/next/src/server/lib/squoosh/impl.ts
+18 more

} catch (e: unknown) {
if (isError(e) && e.code === 'MODULE_NOT_FOUND') {
throw new Error(
'Module `sharp` not found. Please run `npm install --cpu=wasm32 sharp` to install it.'
Copy link
Member

@ijjk ijjk Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we recommending wasm32 here, couldn't another platform be supported and it hasn't been installed yet?

Also what do we think about auto-installing for the user similar to our verifyTypeScriptSetup handling?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since my PR added sharp to packages/next/package.json, it means that the only time we will failing during require('sharp') is the time when it failed to install. The only time it will fail to install is if there is not matching prebuilt binary and it can't build from source. In that case, it seems its likely the current platform would support wasm, so we print this helpful error message.

I don't think typescript has native vs wasm variants so that use case is a little different. We also don't list typescript under optionalDependencies.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at verifyTypeScriptSetup and I think we could follow a similar pattern to automatically install

const packageManager = getPkgManager(baseDir)

But we don't know for sure if installing wasm will work so we might get into an infinite loop.

I checked with the two big providers, Codesandbox and Stackblitz, and both indicate that this should work automatically without the user manually installing wasm variant.

@styfle styfle merged commit 07c4ec0 into canary Feb 6, 2024
66 checks passed
@styfle styfle deleted the remove-squoosh-add-sharp-optionaldep branch February 6, 2024 19:17
@karlhorky
Copy link
Contributor

Oh this is amazing 😍

So if I'm understanding correctly, starting with next@14.1.1-canary.38, there's no need to add sharp as a dependency in package.json anymore...? optionalDependencies are installed automatically by all package managers?

@styfle
Copy link
Member Author

styfle commented Feb 6, 2024

Correct

@styfle
Copy link
Member Author

styfle commented Feb 7, 2024

I confirmed that canary works in both StackBlitz and CodeSandbox 😌

feedthejim added a commit that referenced this pull request Feb 8, 2024
@styfle styfle restored the remove-squoosh-add-sharp-optionaldep branch February 8, 2024 16:16
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 23, 2024
@styfle styfle deleted the remove-squoosh-add-sharp-optionaldep branch March 15, 2024 13:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: Indicate High memory usage if not installing sharp for image optimization
4 participants