Skip to content

NextJS: Allow disabling next/image lazy loading#21909

Merged
valentinpalkovic merged 5 commits intostorybookjs:nextfrom
martinnabhan:next-image-disable-lazy-loading
Apr 25, 2023
Merged

NextJS: Allow disabling next/image lazy loading#21909
valentinpalkovic merged 5 commits intostorybookjs:nextfrom
martinnabhan:next-image-disable-lazy-loading

Conversation

@martinnabhan
Copy link
Copy Markdown
Contributor

@martinnabhan martinnabhan commented Apr 4, 2023

What I did

next/image lazy loads images by default. When using Storybook (and especially Chromatic) we usually want images below the fold to load as well so they show up in snapshots. This pull request sets next/image's loading property to eager which disables lazy loading.

How to test

  1. Create a story using next/image and place an image below the fold
  2. Open Storybook in your browser
  3. Access the story and make sure the image gets loaded even if it's below the fold (or use Chromatic to snapshot the story and make sure images below the fold are loaded and visible)

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@shilman shilman requested a review from valentinpalkovic April 5, 2023 01:44
@valentinpalkovic
Copy link
Copy Markdown
Contributor

valentinpalkovic commented Apr 5, 2023

@martinnabhan Thank you so much for your contribution.

Your PR would introduce a breaking change where people might rely on lazy loading behavior. Instead, this option should be configurable. Let's add it to the parameters.nextjs object instead, where people like you could define the default loading behaviour.

@martinnabhan
Copy link
Copy Markdown
Contributor Author

martinnabhan commented Apr 5, 2023

I thought that might be the way to go! I'll see if I can turn it into a parameter some time this week.

@martinnabhan martinnabhan marked this pull request as draft April 5, 2023 06:35
@martinnabhan martinnabhan force-pushed the next-image-disable-lazy-loading branch 2 times, most recently from 6ab3a72 to e06f67e Compare April 13, 2023 05:51
martinnabhan
martinnabhan commented Apr 13, 2023
View reviewed changes
Comment thread code/e2e-tests/framework-nextjs.spec.ts
martinnabhan
martinnabhan commented Apr 13, 2023
View reviewed changes
Comment thread code/frameworks/nextjs/src/images/decorator.tsx
martinnabhan
martinnabhan commented Apr 13, 2023
View reviewed changes
Comment thread code/frameworks/nextjs/src/images/next-image-stub.tsx Outdated
martinnabhan
martinnabhan commented Apr 13, 2023
View reviewed changes
Comment thread code/frameworks/nextjs/template/stories/Image.stories.jsx
martinnabhan
martinnabhan commented Apr 13, 2023
View reviewed changes
Comment thread code/frameworks/nextjs/template/stories/Image.stories.jsx
@martinnabhan martinnabhan force-pushed the next-image-disable-lazy-loading branch from e06f67e to fc49e62 Compare April 13, 2023 06:37
martinnabhan
martinnabhan commented Apr 13, 2023
View reviewed changes
Comment thread code/frameworks/nextjs/src/images/context.ts
martinnabhan
martinnabhan commented Apr 13, 2023
View reviewed changes
Comment thread code/frameworks/nextjs/README.md
@martinnabhan martinnabhan marked this pull request as ready for review April 13, 2023 06:55
@valentinpalkovic valentinpalkovic added the ci:daily Run the CI jobs that normally run in the daily job. label Apr 17, 2023
valentinpalkovic
valentinpalkovic reviewed Apr 17, 2023
View reviewed changes
Comment thread code/frameworks/nextjs/src/images/next-image-stub.tsx Outdated
valentinpalkovic
valentinpalkovic reviewed Apr 17, 2023
View reviewed changes
Comment thread code/frameworks/nextjs/src/images/next-image-stub.tsx Outdated
@martinnabhan martinnabhan force-pushed the next-image-disable-lazy-loading branch from fc49e62 to 0d64367 Compare April 17, 2023 07:07
@martinnabhan martinnabhan changed the title Disable next/image lazy loading Allow disabling next/image lazy loading Apr 17, 2023
@martinnabhan martinnabhan force-pushed the next-image-disable-lazy-loading branch from ca42384 to 614b8ef Compare April 25, 2023 01:19
@martinnabhan
Copy link
Copy Markdown
Contributor Author

martinnabhan commented Apr 25, 2023

@valentinpalkovic Thank you for approving! What's the status on this? build-sandboxes seems to be failing, but it doesn't seem to be related to this pull request.

@valentinpalkovic valentinpalkovic force-pushed the next-image-disable-lazy-loading branch from 614b8ef to 0a29a0b Compare April 25, 2023 10:21
@valentinpalkovic valentinpalkovic merged commit a333319 into storybookjs:next Apr 25, 2023
@shilman shilman added maintenance User-facing maintenance tasks and removed feature request labels Jul 4, 2023
@shilman shilman changed the title Allow disabling next/image lazy loading NextJS: Allow disabling next/image lazy loading Jul 4, 2023
@shilman shilman added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Jul 4, 2023
@shilman shilman mentioned this pull request Jul 4, 2023
Merged
18 tasks
shilman pushed a commit that referenced this pull request Jul 4, 2023
@valentinpalkovic @shilman
Merge pull request #21909 from martinnabhan/next-image-disable-lazy-l…
533bc69 
…oading

Allow disabling next/image lazy loading
@github-actions github-actions Bot mentioned this pull request Jul 5, 2023
Merged
27 tasks
@JReinhold JReinhold added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jul 5, 2023
@martinnabhan martinnabhan mentioned this pull request Aug 18, 2023
Merged
5 tasks
@dannyhw dannyhw mentioned this pull request Feb 5, 2024
Merged
8 tasks
@ndelangen ndelangen removed the patch:yes Bugfix & documentation PR that need to be picked to main branch label Oct 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@valentinpalkovic valentinpalkovic valentinpalkovic approved these changes

Assignees

@valentinpalkovic valentinpalkovic

Labels

ci:daily Run the CI jobs that normally run in the daily job. maintenance User-facing maintenance tasks nextjs patch:done Patch/release PRs already cherry-picked to main/release branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@martinnabhan @valentinpalkovic @shilman @ndelangen @JReinhold