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

Non imgix urls no longer supported. #801

Open
thejuan opened this issue Jun 17, 2021 · 10 comments
Open

Non imgix urls no longer supported. #801

thejuan opened this issue Jun 17, 2021 · 10 comments

Comments

@thejuan
Copy link

thejuan commented Jun 17, 2021

Upgrading from 9.0 to 9.1 relative urls and non-imgix urls are no longer supported. We used to pass in our image value and if it was imgix great, if it wasnt it will still render normally.

This is currently preventing us upgrading as we would have go and wrap all imgix controls with a conditional render of something else. It'd be great if this behaviour was still available

@sherwinski
Copy link
Contributor

Hey @thejuan, thanks for opening this issue.

Could you please provide a reproducible example for both relative and non-imgix URLs? My understanding here is that the features you are describing were never explicitly supported by this project.

@thejuan
Copy link
Author

thejuan commented Jun 23, 2021

Here is an example repo https://github.com/thejuan/imgix

I haven't styled it, but you can see the image being loaded as a relative background

image

@michaelschufi
Copy link

I'm also interested in using the same component, i.e. Imgix, for local development. This way, I can avoid yet another "slight" difference to production environments.

So what I mean is, to use https://localhost:1234/[image] instead of https://[your_domain].imgix.net/[image] in the src prop.

@frederickfogerty
Copy link
Contributor

@michaelschufi thanks for the comment. I'm just curious how your local setup works - does it proxy to imgix then? The curiosity we have with these use cases is that if you're not using imgix URLs, the imgix parameters will have no effect and so the image will behave differently in development and production.

@michaelschufi
Copy link

@frederickfogerty Thanks for your response. I hope I can clarify my use case:

I have a local docker-compose setup which makes my whole environment, frontend, backend, s3 bucket, etc., available on localhost. Everything is completely local and therefore has very low latency and is available even when my system is offline.

For images, I simply ignore the query parameters when serving the image locally. But I can still check, what query parameters the library would generate and how the component behaves. If I later want to test e.g. loading times or how it actually looks, I can either deploy it to a test environment, or change the base URL used in the src prop with a simple variable change. It would give me full flexibility.

You are right of course by saying that the image itself will render in a different quality. But the behavior of the app will be the same for development and production.

@frederickfogerty
Copy link
Contributor

Thanks for the explanation @michalschufi. That is definitely an interesting use case. I will note your use case down and if we have some free we will see if we can get to it. Unfortunately I can't promise any concrete timelines any time soon - this thread will be updated when we have more information.

@Thomas-Cake
Copy link

Seeing the same issue now after an upgrade. I don't need Imgix to actually serve the image on localhost, understood why that's an issue. But it'd be great if the component wouldn't throw and error and therefore prevent rendering of the whole page, it seems better to just fail silently and produce a broken image instead. Is that an option we can configure somewhere?

@bradcerasani
Copy link

Just ran into this too. Looks like the change was introduced here: 690e7b6

// src/constructUrl.js
function extractClientAndPathComponents(src) {
  const [scheme, rest] = src.split("://");
  const [domain, ...pathComponents] = rest.split("/");
  // etc.

If src does not include ://, scheme contains all of src and rest is undefined. When rest is split on the following line, JS throws with Cannot read property 'split' of undefined.

Confirmed reverting to v9.0.3 works as expected.

Use case:

Screen Shot 2022-01-23 at 21 34 02

@frederickfogerty
Copy link
Contributor

@bradcerasani thanks for thoroughly detailing the issue you're facing, and @Thomas-Cake thanks for responding to this issue. As there is starting to be more support for resolving this issue, I will bring it up with my team again and see if we can prioritise this. If anyone else is reading this, please give the original issue a 👍 reaction or reply below to show that it's also affecting you.

@frederickfogerty
Copy link
Contributor

Hi everyone! We've discussed this internally since there was support shown for this issue, and we are happy to "support" non-imgix urls for the intent of easier development. The implementation will pass down the url value if it is unable to be parsed, with an accompanying warning. To be clear, we are not going intending to support non-imgix urls in production, but we understand that there are development workflows where images can be locally proxied or similar workflows. We will get round to implementing this at some point, but we would also welcome a PR for this if someone would like to take it on!

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