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

Issue with NextJS 10 #219

Open
ScreamZ opened this issue Nov 13, 2020 · 14 comments
Open

Issue with NextJS 10 #219

ScreamZ opened this issue Nov 13, 2020 · 14 comments

Comments

@ScreamZ
Copy link

ScreamZ commented Nov 13, 2020

Have a look at this thread vercel/next.js#18946

@guillaumeLamanda
Copy link

guillaumeLamanda commented Nov 15, 2020

Hello there! First, thank's you for the work made here.

I hope to add more context to this issue.

The problem

I am trying to bump my project to nextJS 10.
This project use next-optimized-images in the latest stable version.
The build break with the following error :

Error: Cannot find module '<project-path>/node_modules/responsive-loader/lib/cjs.js/sharp'

Indeed, in responsive-loader.js#l11, detectedLoaders.responsive is equal to <project-path>/node_modules/responsive-loader/lib/cjs.js. Notice I don't know if this is intended.
After analyzing the structure, I suppose It should be <project-path>/node_modules/responsive-loader/lib/adapters.

I think I ran into this because nextJS 10 bring sharp dependency.
I was previously using sharp, but the responsive loader seems to detect sharp first.

Solutions

Fix on the version 2

This was my chosen solution. I opened a pull request for it. It is minimal, and all checks are OK.
I hope a maintainer will find the time to review it.

Upgrade to the version 3 canary

I tried this but ran into #195

@ScreamZ
Copy link
Author

ScreamZ commented Nov 15, 2020

Thanks 😅

@SalahAdDin
Copy link

Is this still useful with the new version?

@guillaumeLamanda
Copy link

guillaumeLamanda commented Nov 23, 2020

Is this still useful with the new version?

With the alpha 3 version, I encounter #195. Version 3 will be a good solution when stable 😃

@ScreamZ
Copy link
Author

ScreamZ commented Nov 23, 2020

@SalahAdDin It is, to import from modules

@SalahAdDin
Copy link

@ScreamZ I mean, is this page still relevant with the new NextJS image api?

@ScreamZ
Copy link
Author

ScreamZ commented Nov 23, 2020

Yeah, because NextJS don't do the same things exactly, they are optimizing other things

@SalahAdDin
Copy link

is there any tutorial about that?

@guillaumeLamanda
Copy link

guillaumeLamanda commented Nov 23, 2020

As said in the NextJS documentation

Instead of optimizing images at build time, Next.js optimizes images on-demand, as users request them.
Unlike static site generators and static-only solutions, your build times aren't increased, whether shipping 10 images or 10million images.

For my personal use case, I need to build time URL generation at least to fill product metadata.
Later, I think I can deduce the image URL from the image path, but I would like NextJS to provide a function for this purpose.

@ScreamZ , from my point of view, NextJS Image component and this library do the same things, but not at the same time.
Notice that the way NextJS is doing it is more scalable because images are optimized "on the fly", which reduce drastically the build time.

This still has drawbacks.
First, you're using a managed service so optimize the images for you. This means: you need vercel (or another loader) for an optimized experience with NextJS <Image/>.
Then, we can imagine a higher response time when calling for an image for the first image call. Later, vercel will leverage caching.

EDIT: It also works with a custom server as specified on the loaders page section.
The confusion was initiated because I deploy on vercel, so it uses the vercel image loader.

@JbPons
Copy link

JbPons commented Nov 24, 2020

This still has major drawbacks.
First, you're using a managed service so optimize the images for you. This means: you need vercel for an optimized experience with NextJS. Notice you can use another image endpoint to use another service, but it needs some extra work on your side.
Then, we can imagine a higher response time when calling for an image for the first image call. Later, vercel will leverage caching I suppose.

@guillaumeLamanda I'm not sure to understand, we must use vercel to work with the new image component of next.js ?
In my case, 80% of my images are svg and the rest are png and gif and i have some uploaded images that come from a rest API. Basically i want to optimize the png and gif part. I don't really know if it's revelant to optimize the svg (and if you can explain me the benefit of optimizing svg i will be interested). And I don't use vercel i have an another server for this project.

When i read the next.js documentation and when i saw this library, I was thinking about wich solution is better for my current project. So, if you can help me to take the good decision i will be very happy 😄

@durchanek
Copy link

durchanek commented Nov 26, 2020

I'm not sure to understand, we must use vercel to work with the new image component of next.js ?

I do not think so, the documentation states the built-in optimization is used by default.

@guillaumeLamanda
Copy link

@durchanek thank's I missed the default loader line. I updated the comment to clear the confusion it brings.

So @JbPons, the <Image/> component does not require to use Vercel. The loader page pointed above says it works with a custom server. You still need to be careful with your max-age header

@JbPons
Copy link

JbPons commented Nov 26, 2020

Thanks for the clarification

@lansolo99
Copy link

I first implemented the workaround proposed here before realizing there is a better way recommended to fix this using the 'responsive' property in next.config.js. Reference: #206

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants