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

Node 10.x #157

Closed
wants to merge 2 commits into from
Closed

Node 10.x #157

wants to merge 2 commits into from

Conversation

jedsmith13
Copy link

Issue #, if available: 156

Description of changes: Adjusted the template to use nodejs10.x and updated sharp

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

This is to ensure it works with the latest NodeJS.
@jedsmith13 jedsmith13 mentioned this pull request Oct 21, 2019
@OBrown92
Copy link

Do you test it? I think it won't work because there are breaking changes like new Buffer with Stability: 0. If i set my running Version to 10.x, it will get me a internal Server Error.

@jedsmith13
Copy link
Author

jedsmith13 commented Oct 24, 2019 via email

@OBrown92
Copy link

Nice, I will try. Just took a quick look a few days ago and found these issues, seems to be fixed with the sharp upgrade.

@jedsmith13
Copy link
Author

@OBrown92 Also while following the custom build I had to follow this comment. If you run into issues with not being able to find to your bucket with the source code when you run your cloud formation template, that comment helped me.

@OBrown92
Copy link

Thanks, I try to create a zip file like the one from ilove because I also need the changes. Unfortunately it won't work by now.

@jedsmith13
Copy link
Author

@OBrown92 Sorry, I don't know anything about ilove's build. I would suggest you contact the maintainer of that project if you want help with it.

@mberneis
Copy link

@jedsmith13 - Could you share your template URL of a successful build? -- I am facing issues creating a proper cloud formation build (#115 (comment))

@jedsmith13
Copy link
Author

I didn't upload my template to S3 I just uploaded it when I created a new cloud formation instance. I just uploaded a copy though I don't know how long I will keep the image-handler.zip file on the S3 bucket. I figure the AWS one will get updated in the next month or two and if not I will probably customize the one I use some.

@mberneis
Copy link

I could not run a cloud-formation with your template - bucket not found - I guess the code bucket does not exist anymore. - At this point I will wait a month to see if AWS updates their solution - otherwise back to debugging!

@beomseoklee
Copy link
Member

Thanks for the pull request, and we are trying to update the solution to support Node.js 10.x in this year.

@georgebearden
Copy link
Member

Hi All - This solution has been updated to a later version of Node. Please let us know if you need additional assistance.

@rpong
Copy link

rpong commented Dec 18, 2019

Thanks, I try to create a zip file like the one from ilove because I also need the changes. Unfortunately it won't work by now.

Hope this helps, releasing the same pre-package we are using for Node 12.x.

https://github.com/innovationlove/serverless-image-handler/releases/tag/4.0.3

Unfortunately though, we are using Sharp 0.22 instead of 0.23, there are some cropping/fitting behaviour in 0.23 that is different with 0.22 that we have not looked into yet.

Example: dimensions such as 150x0 work with 0.22, but fails in 0.23. We will look into it soon and try to share our changes.

@mberneis
Copy link

Had the same issue with resizing proportinally by setting 0 to one dimension. - Figured out it works if you specify null instead of 0

thisismana pushed a commit to thisismana/serverless-image-handler that referenced this pull request Oct 10, 2022
* feat: First attempt to allow avif

* fix: try to fix formatting

* fix: add regex and error message

* fix: add error text to test
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.

6 participants