chore(gatsby): convert sanitize-node to typescript #36327
Conversation
gatsbot
bot
added
the
status: triage needed
marvinjude
added
topic: TypeScript
| @@ -20,15 +68,15 @@ const sanitizeNode = (data, isNode = true, path = new Set()) => { | |||
| returnData[key] = o | |||
| return | |||
| } | |||
| returnData[key] = sanitizeNode(o, false, path) | |||
| returnData[key] = sanitizeNode(o as data, false, path) | |||
There was a problem hiding this comment.
Can we change this loop to a Object.entries(data).forEach? That should allow TS to pick up the right types and we shouldn't need to cast.
There was a problem hiding this comment.
Object.entries hardcodes the first value o as string, making tests fail (Symbol/string issue, .each avoids this issue).
|
|
||
| if (returnData[key] !== o) { | ||
| anyFieldChanged = true | ||
| } | ||
| }) | ||
|
|
||
| if (anyFieldChanged) { | ||
| data = omitUndefined(returnData) | ||
| data = omitUndefined(returnData as data) as data |
There was a problem hiding this comment.
If we can type returnData during initialization, we should be able to get away with no casting here.
There was a problem hiding this comment.
returnData is initalised as {} or [].
{}is not a validIGatsbyNodeerror: Type '{}' is missing the following properties from type 'IGatsbyNode': id, parent, children, internal, fields[]is not a validGatsbyIterable<IGatsbyNode>error:
Type 'never[]' is missing the following properties from type 'GatsbyIterable': source, deduplicate, mergeSorted, intersectSorted, deduplicateSorted
I could cast when we define returnData to avoid the double casting here, but we would still need one as omitUndefined returns a Partial<data> (from lodash, pickBy returns a Partial).
I think it's down to personal preference here, I feel the double cast here is more significant than having it when we define returnData but I'm open to change it.
There was a problem hiding this comment.
Sounds reasonable enough. Thank you for the detailed response. 😄
…ert-sanitize-node-to-typescript
…il/gatsby into convert-sanitize-node-to-typescript
|
Code looks good! Got builds kicked off and I'll check back later. 😄 |
|
Thanks again @Kornil . We appreciate your patience and all of your contributions to Gatsby 💜 |
Convert sanitize-node to TS.
Related Issues
#21995