Help avoid unnecessary hits to process & invalid requests#5
Conversation
1. Unnecessary hits to process are any hits that have no resize options. When the helper is used without resize options, it should simply return either the URL passed to it (if passed a URL), or a URL to https://cdn.filestackcontent.com/token 2. Prevent invalid requests by ensuring either `width` or `height` is present in the resize options. Otherwise return simple image url.
This commit adds support for `ENV.filestackProcessCDN` which should proxy to https://process.filestackapi.com and `ENV.filestackContentCDN` which should proxy to https://cdn.filestackcontent.com. Setting up these CDNs (on AWS Cloudfront, for instance) will save duplicate hits to transform & display, making your filestack quotas last longer.
|
Should probably update the readme to mention the CDN configuration option... |
| imageUrl = `https://cdn.filestackcontent.com/${token}`; | ||
| urlRoot = 'https://process.filestackapi.com'; | ||
| } | ||
| Object.keys(hash).forEach((key) => { |
There was a problem hiding this comment.
shouldn't this forEach move to within the if at current line 36? No need to process if we don't have enough options, right?
There was a problem hiding this comment.
Yes good point! Updating incoming
|
@mminkoff there are indeed other processing tasks besides resizing, it's just that this helper has I'll add something to the readme presently... |
Avoid looping through the hash if it doesn't have the required options (width or height).
|
To your other point -- the other transformations filestack supports -- it would be awesome to add support for them to the helper (or maybe add other helpers?). I think that should be done in a separate PR, maybe after some discussion about how to support them. If we support them all in this helper, the helper is likely to be ridiculously large. Maybe we add options parsing per transformation type in utilities, or maybe we make each one its own helper (with some shared utilities). I'm not really sure what the best approach is. But I know it would be sweet to add support for image-to-ascii transformations 😀 |
|
@sdhull sorry for the delay. Great updates - thanks! As for supporting all options, it's only an issue because we're checking for what's there. I'd be more inclined to leave that up to the developer so they can put whatever options they want. Maybe we should at least rename the helper to something specific to resize, though I don't want to introduce breaking changes. I'll go with this for now and would welcome any support in broadening what this can do. Thanks so much! |
2 participants
widthorheightis present in the resize options. Otherwise return simple image url.