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

Added support custom domains for images #208

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mihailShumilov
Copy link

added support for custom domain for images
to use custom domain you should set config param nextImageExportOptimizer_imagesDomain

to use custom domain you should set config param `nextImageExportOptimizer_imagesDomain`
@mihailShumilov
Copy link
Author

@Niels-IO - 1 month without any review...

@Niels-IO
Copy link
Owner

Niels-IO commented Apr 7, 2024

Hi @mihailShumilov,
Thanks for the PR. As this is not my main work, things might take their time.. I will have a look but it might take until May.

@mihailShumilov
Copy link
Author

Hi @mihailShumilov, Thanks for the PR. As this is not my main work, things might take their time.. I will have a look but it might take until May.

If you want - you can add me as a maintainer/collaborator - I can help you with this project

@dannyDNS
Copy link

dannyDNS commented Jun 12, 2024

I was just looking at implementing support for this myself. Have you considered reading the assetPrefix value from the next config?

Edit: the actual problem here IMO is that this package doesn't respect assetPrefix, making it incompatible with other methods of rendering images on the page (i.e. as a background image). If you're using assetPrefix, this package mangles your URLs, making srcSets that are broken.

@Niels-IO
Copy link
Owner

Hi there,

Can you please explain the use case for using custom domains? Regarding assetPrefix: The Next.js documentation specifies:

While assetPrefix covers requests to _next/static, it does not influence the following paths:

Files in the public folder; if you want to serve those assets over a CDN, you'll have to introduce the prefix yourself

So I think this would add the need to handle statically imported images and string paths differently if I were to implement assetPrefix. Also assetPrefix would be a global setting for all assets.

This PR is also mixing two added functionalities: custom image domains and some very user specific custom logic for mobile src's. The mobile src conditional should live in user land not inside this package.

@PhilGale92
Copy link

Just thought I'd add to this - i have a very similar use case to the above - i had a requirement of having the images served with the assetPrefix which is not an actual folder but rather from akmai routing.

Not piling in or anything but its not just a single edge case.

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

Successfully merging this pull request may close these issues.

4 participants