vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.71k stars 26.94k forks source link

blurDataURL inflates bundle even when not used #54012

Open MattIPv4 opened 1 year ago

MattIPv4 commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP PREEMPT_DYNAMIC Sun Aug  6 20:05:33 UTC 2023
    Binaries:
      Node: 16.17.0
      npm: 8.15.0
      Yarn: 1.22.19
      pnpm: 7.1.0
    Relevant Packages:
      next: 13.4.15-canary.0
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.1.3
    Next.js Config:
      output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

Image optimization (next/image, next/legacy/image)

Link to the code that reproduces this issue or a replay of the bug

https://codesandbox.io/p/sandbox/tender-morning-84653l

To Reproduce

Import a large image into any page while image optimization is enabled for the project. For extra points, and extra bundle bloat, import an animated image. Use the image with next/image, but do not enable the blur placeholder.

Build the project and inspect the JS bundle for the page the image is used on.

Describe the Bug

The Webpack loader for images in Next.js always injects a blurDataURL into the JS bundle, even if it is unused in the code. It looks like in 13.4.8 some sort of tree-shaking was added that makes this no longer an issue for the native image tag, but this is still a problem for next/image.

For large images, and especially animated images where the "blur" image is just the original image, this massively inflates the JS bundle. There does not appear to be a way to opt-out of this blurDataURL being generated for a specific image, unless you opt the entire project out of using the image optimization, which means without this being tree-shaken correctly, there is no way to escape this bundle inflation if you want to use next/image.

https://github.com/vercel/next.js/blob/c4f9c6d492d5daf3b470a755eaf3d0f68dd0df08/packages/next/src/build/webpack-config.ts#L2198-L2220

https://github.com/vercel/next.js/blob/c4f9c6d492d5daf3b470a755eaf3d0f68dd0df08/packages/next/src/build/webpack/loaders/next-image-loader/index.ts#L45-L54

https://github.com/vercel/next.js/blob/c4f9c6d492d5daf3b470a755eaf3d0f68dd0df08/packages/next/src/build/webpack/loaders/next-image-loader/blur.ts#L68-L84 (note that if the image is detected as animated, the entire content is used for the base64 blur image, which then ends up directly in the JS bundle)

Expected Behavior

For animated images, it probably makes sense to return an empty blurDataURL, rather than trying to use the entire animated image as the blurDataURL.

For other images, it'd be nice to see a way to opt-out of the blurDataURL being generated by Webpack (perhaps a query param), or for it to be properly tree-shaken when not used, like it is if you use a native image tag.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

styfle commented 1 year ago

For animated images, it probably makes sense to return an empty blurDataURL, rather than trying to use the entire animated image as the blurDataURL.

That sounds reasonable. Do you want to submit a PR?

styfle commented 1 year ago

For other images, it'd be nice to see a way to opt-out of the blurDataURL being generated by Webpack (perhaps a query param), or for it to be properly tree-shaken when not used, like it is if you use a native image tag.

Can you share more about the solution you're thinking of?

I would like to wait for a import assert to reach stage 4 but it was recently changed to with so I'm not confident that it won't change again. https://github.com/tc39/proposal-import-attributes

MattIPv4 commented 1 year ago

For animated images, it probably makes sense to return an empty blurDataURL, rather than trying to use the entire animated image as the blurDataURL.

That sounds reasonable. Do you want to submit a PR?

Sure, I can get a PR open for that later today :)

For other images, it'd be nice to see a way to opt-out of the blurDataURL being generated by Webpack (perhaps a query param), or for it to be properly tree-shaken when not used, like it is if you use a native image tag.

Can you share more about the solution you're thinking of?

I would like to wait for a import assert to reach stage 4 but it was recently changed to with so I'm not confident that it won't change again. https://github.com/tc39/proposal-import-attributes

TypeScript makes this rather annoying due to the limitations with defining types for modules, but I was thinking something to the effect of import image from "./image.png?no-blur" or some other query param, as Webpack would be able to see that. That would be possible with wildcard module declarations in TypeScript, but puts you in a corner as defining more query params would then get really messy.

Using import assertions to pass more context to Webpack definitely feels like a better option than query params for sure, I had not considered that! I've never personally played around with how those get exposed to Webpack, though I assume it can see them?

That being said, I kinda think that maybe a query param into Webpack is a bit of a hack, and that generating the blur placeholder data always is probably fine, assuming it is possible to fix how the data gets tree-shaken when it gets passed into next/image (my knowledge of tree-shaking is not good enough to suggest a fix there beyond a hand-wave-y "tree-shaking").

styfle commented 1 year ago

generating the blur placeholder data always is probably fine

I think so too since its unlikely that the user would import the img but then not use it as a blur placeholder.

We have the opposite problem today which is that users want blur placeholders for non-static imports.

tumenbaev commented 12 months ago

I want to highlight this issue again. In my use case, I have a set of icons (let's say 100) with only a few of them displayed on a page. The exact icon is determined by API response. So I have a map of icons:

import icon1 from './icon1.png'
import icon100 from './icon100.png'
export const { icon1, icon100 };

And a component that wraps next/image and takes icon name as a parameter <Image src={icons[iconName]} /> This ends up with a significant bundle size increase due to blurDataURL even though it's not used.