feat(gatsby-plugin-image): Allow image helpers to take other node types #29625
Conversation
gatsbot
bot
added
the
status: triage needed
ascorbic
added
topic: media
| } | ||
|
|
||
| describe(`The image helper functions`, () => { | ||
| it(`getImage returns the same data if passed gatsbyImageData`, () => { |
There was a problem hiding this comment.
Non-blocking, but I might make a nested describe for getImage since that's technically the object of the next few tests.
|
|
||
| export const isGatsbyImageData = ( | ||
| node: IGatsbyImageData | any | ||
| ): node is IGatsbyImageData => Boolean(node?.images?.fallback?.src) // 🦆 |
There was a problem hiding this comment.
Is this duck comment a question? Boolean is fine, an if statement might be slightly more readable.
There was a problem hiding this comment.
Oh, it's just saying that I'm duck typing here by checking for specific props. Clearly should have better comments!
|
|
||
| type ImageDataLike = FileNode | IGatsbyImageDataParent | IGatsbyImageData | ||
| export const getImage = (node: ImageDataLike): IGatsbyImageData | undefined => { | ||
| if (isGatsbyImageData(node)) { |
There was a problem hiding this comment.
Is there a reason for making isGatsbyImageData and isGatsbyImageDataParent? It seems like checking for existence inside this function would be equivalent? I noticed those functions are exported, are they meant for plugins to be able to use?
There was a problem hiding this comment.
Making them type guard functions allows type narrowing in TS. I exported them so they could be unit tested, but in the end just tested the getImage function itself so I could remove the export.
There was a problem hiding this comment.
Ah ok. Too bad we can't get the benefit without the extra functions. Would make it easier to scan.
There was a problem hiding this comment.
I think it's easier to read with function name!
ascorbic
force-pushed
the
feat/better-image-helpers
branch
from
cc9a193 to
c608155
Compare
|
Danger run resulted in 1 fail and 1 markdown; to find out more, see the checks page. Generated by 🚫 dangerJS |
laurieontech
left a comment
laurieontech
left a comment
There was a problem hiding this comment.
Thanks! Tests are easier to scan now :)
This allows the
getImageandgetSrchelpers to take nodes other thanFilenodes, as long as they include agatsbyImageDatafield, for exampleContentfulAssets, and allowsgetSrcto take imageData directly. It also adds a stack of unit tests, and a similargetSrcSethelper.