feat(assets): Add property to image services to control which properties to use for hashing#8984
Conversation
…ies to use for hashing
🦋 Changeset detectedLatest commit: 991c813 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
bluwy
left a comment
There was a problem hiding this comment.
I don't quite understand how this feature will fix the linked issues. Are we adding new properties to be hashed to fix them, or would they have to set it themselves? If it's the latter, are we able to do it by default instead?
I can see how this would be useful for third-party image services though since ImageTransform has [key: string]: any.
No, we're actually removing properties! Before we'd pass everything that was passed to With this list, we now can know what properties a service actually use to impact the image, so we can add only them to the hash. Third-party services will adjust their list with the properties they need. The alternative is maintaining a blacklist, but it's very unreliable because of data attributes, proprietary attributes from third-party librairies etc. Users don't need to do anything, this is only specific to services, and even then it's only specific to local services that don't use our properties (since the default value is the properties we use ourselves) |
ematipico
left a comment
There was a problem hiding this comment.
Should we have at least a new test case where we use this new option?
sarah11918
left a comment
There was a problem hiding this comment.
Left a quick suggestion to include the property name!
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
|
A small warning (probably related to ths update): I was using the |
Shouldn't be related to this, but if you can reproduce the problem, feel free to open an issue! |
…ies to use for hashing (#8984) Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Changes
What the title says. This is a bit advanced, and most image services won't need it so we supply a good enough default while still enabling more advanced use cases.
Fix #8956
Fix #8047
Testing
The current tests should pass! In particular, we have a test right now that tests image duplication in
srcset(the most complex case). It already passed before, but it should still catch regressionsDocs
Will add it to the Image Service API page