-
Notifications
You must be signed in to change notification settings - Fork 18
feat: Handle pollForProcessingImage breaking change #129
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
base: main
Are you sure you want to change the base?
Conversation
|
@RS-labhub is attempting to deploy a commit to the Cloudinary DevX Team on Vercel. A member of the Team first needs to authorize it. |
|
@devpatocld you can check it now. referring: #51 (comment) |
|
Bumping this up @devpatocld @eeeps |
|
It's been more than a month. Still no action is taken. Edit: I just saw this comment. I'm glad I didn't participate in Hacktoberfest this year. LOL |
|
@RS-labhub I think the issue you claimed to fix, of the pollForProcessingImage result being object was actually already handled correctly, by https://github.com/cloudinary-community/astro-cloudinary/pull/63/files The issue still being open, and us adding the hacktoberfest tags to it, was our fault, and I apologize if we wasted a moment of your time. As for the PR - it seems you have found an unrelated (but close in the source code) issue, which is a mistake in handling query parameters incorrectly. Reading the code, it seems like an obvious fix, however I'm frankly unable to replicate the issues I would think it would cause, and am unsure how this bit of code interacts with the automatic responsive image features and srcset. As you found in the other issue, we have been very slow to review PRs this year, and I sincerely apologize for the delay. I will pick this back up in early December, and my colleague @devpatocld will reach out to you about Hacktoberfest swag. In the meantime, if you have an example case where us tacking the key onto the URL with |
|
Hey @eportis-cloudinary, thank you for your kind gesture. Yes, I saw the source code, and the issue has already been solved but there was just incorrect or missing parameter logic handling there so I fixed it. And for your question:
The answer is, it doesn't cause issue but there might be an edge case if the url uses both The current implementation handles both cases properly:
And ig this is the standard way to append query parameters to URLs. |
Description
Updates CldImage to handle the breaking change in
@cloudinary-util/utilwherepollForProcessingImagenow returns aPollForProcessingImageResponseobject instead of a boolean.Issue Ticket Number
Fixes #51
Type of change
Checklist