fix(gatsby-source-contentful): Improve base64 placeholders #29034
Conversation
axe312ger
added
the
status: needs core review
gatsbot
bot
added
the
status: triage needed
axe312ger
changed the title
axe312ger
force-pushed
the
perf/contentful-better-lqip-base64
branch
from
29746c1 to
d529588
Compare
axe312ger
added
topic: source-contentful
| ) | ||
| reject(e) | ||
| }) | ||
| const promise = new Promise(async (resolve, reject) => { |
There was a problem hiding this comment.
There shouldn't be a need to wrap this all in a Promise. It's an async function so it returns a promise anyway, and you can just await the fs-extra calls.
There was a problem hiding this comment.
We need the promise for the caching layer. Otherwise I'd have removed it
There was a problem hiding this comment.
There was a problem hiding this comment.
Yeah, but async fucntions return promises anyway, so no need to wrap it like you've done.
There was a problem hiding this comment.
async function loadImage() {..then:
const promise = loadImage()There was a problem hiding this comment.
Well yeah I can make a small function that is called. I tried not to alter the code structure to much.
| const body = `data:image/jpeg;base64,${base64}` | ||
|
|
||
| // TODO: against dogma, confirm whether writeFileSync is indeed slower | ||
| fs.promises |
There was a problem hiding this comment.
You have fs-extra in the project which promisifies all of the fs calls, so you can just use that.
There was a problem hiding this comment.
But why go fs-extra if we have it supported in the language?
There was a problem hiding this comment.
It is marked as stable api:
https://nodejs.org/dist/latest-v10.x/docs/api/fs.html#fs_fs_promises_api
ascorbic
changed the title
This replaces the no more maintained
base64-imgpackage with a more native implementation.Additionally it fixes the following bug:
This should fix the security vulnerabilities from #24679