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

[canary] Using sizes prop triggers non-fatal error #169

Closed
brandonpittman opened this issue Jul 5, 2020 · 10 comments
Closed

[canary] Using sizes prop triggers non-fatal error #169

brandonpittman opened this issue Jul 5, 2020 · 10 comments

Comments

@brandonpittman
Copy link

brandonpittman commented Jul 5, 2020

Any time I apply the sizes prop, I get this. Given enough time, the build will proceed. Only seems to happen once per server run, not on each page visit. Build unaffected, so deploys are possible. Just causes a weird slow down any time the dev server is spun up.

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 
11 uncaughtException listeners added to [process].
Use emitter.setMaxListeners() to increase limit
@brandonpittman
Copy link
Author

I gave up and stopped using the sizes prop for now. Build speeds immediately faster and the warning has gone away.

@cyrilwanner
Copy link
Owner

Hi @brandonpittman
I just found and fixed a bug in the cache handling which re-compiled images instead of loading them from the cache. That led to the slow dev server you mentioned. It is now fixed in version 3.0.0-canary.2.

Regarding the message, I'll try to also get rid of it. If you have 4 images and want to provide 3 sizes each, that already results in 12 images. And since those 12 images get compiled parallel, this warning comes up. But I'll see what I can do here.

(After updating, it is expected for all images to re-compile once)

@brandonpittman
Copy link
Author

brandonpittman commented Jul 24, 2020

@cyrilwanner Does seem a bit faster with canary.2.

However, this issue is now coming up.

Error: Babel plugin 'react-optimized-image/plugin' not installed or this component could not be recognized by it.

Only seems to be happening with images that have dynamic sources. Those dynamic sources worked with canary.0.

Yes, the plugin is defined in .babelrc.

{
  "presets": ["next/babel"],
  "plugins": ["react-optimized-image/plugin"]
}

@cyrilwanner
Copy link
Owner

Sorry for the error, that looks like a bug as dynamic sources should work generally.
Can you share a code snipped that is not working and produced that error?

@brandonpittman
Copy link
Author

@cyrilwanner This is the component that was bugging out. The image stuff is currently commented out due to the error I was getting.

https://github.com/brandonpittman/next-blog/blob/master/src/components/media_card.js

@DevinEdwards
Copy link

@cyrilwanner great work. Looking forward to using this moving forward. 🙌

@cyrilwanner Does seem a bit faster with canary.2.

However, this issue is now coming up.

Error: Babel plugin 'react-optimized-image/plugin' not installed or this component could not be recognized by it.

Only seems to be happening with images that have dynamic sources. Those dynamic sources worked with canary.0.

Yes, the plugin is defined in .babelrc.

{
  "presets": ["next/babel"],
  "plugins": ["react-optimized-image/plugin"]
}

I can confirm this as well. The dynamic paths return an error while the static path succeeds

Error

const filename = 'image.jpg'
<Img src={require(`@images/${filename}`)} />

Error: Babel plugin 'react-optimized-image/plugin' not installed or this component could not be recognized by it.

Success:

  <Img src={require(`@images/image.jpg`)} />

The same is true with/without the @images alias.

Interestingly when I found this: webpack/webpack#6680 (comment). Looks like the filename can be dynamic but the extension can not?

Success:

const filename = 'image'
<Img src={require(`@images/${filename}.jpg`)} />

For now, I plan on making a workaround to check the extension on the image, as I am already getting the image data in static props to get the ratio to prevent layout shifting.

Looking forward to learning more about this, and again thanks for your work.

@cyrilwanner
Copy link
Owner

Thank you both for the feedback! It was indeed a bug in next-optimized-images and should be resolved in the 3.0.0-canary.4 version. So even if the extension is dynamic/in the variable, it should now work again. Is it possible for you to test the new canary version and confirm that? Thank you!

Also, please note that if you are using a dynamic require, all images matching the path before the variable get compiled because webpack does not know during build which values the variable could have. So if you have images/${name} and specify sizes={[200, 400]}, it creates 200px & 400px versions of all images within the images/ folder which could drastically increase build time if only a few of those images are actually used in those sizes. So if you only need 200px & 400px versions of a few images, it would make sense to move those into their own subfolder, e.g. images/products/.
I understand that this is not required for your blog since you will display all posts the same way, I just wanted to point this out in case someone else lands here with an increased build time with dynamic requires :)

@brandonpittman
Copy link
Author

brandonpittman commented Jul 28, 2020

@cyrilwanner I can confirm the errors are gone. Can you recommend a better dynamic image solution? I'm wondering how Gatsby Image and Gridsome's image component manage to be so quick when converting images with Sharp.

@cyrilwanner
Copy link
Owner

In general, as mentioned above, splitting your images into different subfolders (which are not part of the dynamic variable) improves it a lot.

Also, next-optimized-images should cache your images so only the first build or when you change sizes should take long. If everything gets loaded from cache, it should only take a few milliseconds. Only if you update next-optimized-images, it could happen that the cache gets invalidated (when something related to image optimization or caching got changed).
In CI, make sure to cache your node_modules folder (as the cache is located within it). Basically every CI provider has that feature.

Sharp is generally way faster but provides worse results compared to the image optimization tools used in next-optimized-images (regarding the image size with similar optimization levels). I was also thinking if I should use sharp for the canary version, but I think, in the end, your end-users will be more thankful if your builds take 1min longer and in return they have to download less data (which also leads to better web vital results). And when the images are cached, optimization time doesn't really make any difference.

@brandonpittman
Copy link
Author

@cyrilwanner Wow, the last update fixed my issues and the builds seem super fast again. (Cache is working its magic.) Closing this. 🥳

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

3 participants