-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
options.logo support string array #332
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
My only concern is that only first file's hash is used for caching. Cache invalidation is known to be a hard thing ;).
May you check if this change #334 can be useful here?
src/index.js
Outdated
// Recompile filesystem cache if any source based path change: | ||
(fileSources) => | ||
getRelativeOutputPath( | ||
fileSources[0].hash, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to hash all files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Smth like getContentHash(...fileSources.map(s => s.content))
should work.
src/index.js
Outdated
const resolvedPublicPath = getResolvedPublicPath( | ||
logo.hash, | ||
logoFileSources[0].hash, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hash all files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amended 👍
favicons >= 7.0.0 picks source image by it's size, solves jantimon#265
9f781aa
to
c3aeb7d
Compare
thanks the review 👍 let me know if the changes work, and i'll go ahead and update/rebase the maskable pr |
c3aeb7d
to
fd86520
Compare
ah took me a bit to wrap my head around the internals ;) thanks for the example 👍 updated commit pushed |
Thanks! |
favicons >= 7.0.0 picks source image by it's size if provided with an array
solves #265