vercel / next.js

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

next/image preload not working as expected in AppDir. Affecting LCP and FP. Order of preload is different than page-dir. #52995

Closed gauravsaini964 closed 1 year ago

gauravsaini964 commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.5.0: Thu Jun  8 22:22:19 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T8103
    Binaries:
      Node: 16.20.0
      npm: 8.19.4
      Yarn: 1.22.19
      pnpm: 8.6.0
    Relevant Packages:
      next: 13.4.11
      eslint-config-next: 13.4.11
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.1.6
    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://github.com/gauravsaini964/image-preload-next13

To Reproduce

  1. Build the repo.
  2. Navigate to index page (which is using app-dir) and inspect index.html code.
  3. Image preload statement is after font,css,webpack and 2-3 other chunks.
  4. Navigate to /page-dir (which is using page dir) and repeat step 2 & 3.
  5. Image preload is before all other preload tags.

Describe the Bug

Order of image preload tag in app-dir is different from what it used to be in pages-dir and hence FP and LCP metric is getting affected. Same is replicated in lighthouse scores. Compare following screenshots:

Screenshot 2023-07-21 at 4 21 41 PM Screenshot 2023-07-21 at 4 23 43 PM Screenshot 2023-07-21 at 4 34 31 PM Screenshot 2023-07-21 at 4 30 13 PM Screenshot 2023-07-21 at 4 33 29 PM Screenshot 2023-07-21 at 4 31 05 PM

Expected Behavior

Image preload tag should come above every other preload tag imo as it is giving me best performance.

I have a ecommerce app which has 1800+ pages built with both app-router and page-router. App router app is faster in every aspect apart from FP and LCP metric consistently by ~1seconds because that website has lot more scripts to preload.

Which browser are you using? (if relevant)

Chrome

How are you deploying your application? (if relevant)

Vercel

NEXT-1463

gauravsaini964 commented 1 year ago

Any update or ETA on this? Sitting on website that needs to go live.

Thanks for your efforts.

styfle commented 1 year ago

This is an issue in React.

I notified @gnoff who is going to fix it upstream.

gauravsaini964 commented 1 year ago

Hey @gnoff. Any issue, PR or ETA to track?

Thank you for your contributions.

gauravsaini964 commented 1 year ago

Hi Please share if you have any update. Sorry for rushing. One of the last few bugs pending for our new website rollout.

gnoff commented 1 year ago

No issue just yet but I think a general timeline of 2 weeks is reasonable. I'll try to update this issue with a link tracking the work once I get it started

gauravsaini964 commented 1 year ago

Please check #53574. If it is related or not.

gauravsaini964 commented 1 year ago

Hey @gnoff Any progress on this? Thanks

styfle commented 1 year ago

I believe this is the upstream React PR:

steve-marmalade commented 1 year ago

Do we expect that this issue has been fixed since the upstream react fixes were included in 13.4.15 via https://github.com/vercel/next.js/pull/53881 ?

styfle commented 1 year ago

Confirmed this is fixed 👍

github-actions[bot] commented 1 year ago

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.