chore(gatsby) Migrate db methods to Typescript #22725
Conversation
|
We recently disabled This cause bit of conflicts in your PR - would you be able to fix those conflicts? |
| @@ -47,7 +47,7 @@ import getSslCert from "../utils/get-ssl-cert" | |||
| import { slash } from "gatsby-core-utils" | |||
| import { initTracer } from "../utils/tracer" | |||
| import apiRunnerNode from "../utils/api-runner-node" | |||
| import db from "../db" | |||
| import * as db from "../db" | |||
There was a problem hiding this comment.
If possible I prefer to be explicit in the imports.
| import * as db from "../db" | |
| import {saveState, startAutosave} from "../db" |
packages/gatsby/src/db/nodes.ts
Outdated
| import _ from "lodash" | ||
| import { Node } from "gatsby" | ||
| import { store } from "../redux" | ||
| import * as reduxNodes from "../redux/nodes" | ||
|
|
||
| if (process.env.GATSBY_DB_NODES === `loki`) { |
There was a problem hiding this comment.
This file should be much simpler to convert now that loki is dropped.
| @@ -1,12 +1,43 @@ | |||
| const _ = require(`lodash`) | |||
| import _ from "lodash" | |||
| import { Node } from "gatsby" | |||
There was a problem hiding this comment.
Isn't this an IGatsbyNode? From gatsby/packages/gatsby/src/redux/types.ts
| import { Node } from "gatsby" | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| type ObjectOrArray = Record<string, any> | Array<any> | Node |
There was a problem hiding this comment.
Would unknown be a better fit here? I'm not sure if we can refine optional to non-optional here (or somehow exclude void from the typing) but that's probably not very useful.
| */ | ||
| const sanitizeNode = (data, isNode = true, path = new Set()) => { | ||
| export function sanitizeNode<T extends Node>( |
There was a problem hiding this comment.
What is T when isNode is false?
The recursive call to this function might receive a node (IGatsbyNode) but most likely will be a plain object or filter value.
| data: T, | ||
| isNode: boolean = true, | ||
| path: Set<T> = new Set() | ||
| ): T | undefined { |
There was a problem hiding this comment.
Maybe the initial call to this recursive function should be separated (fine from a perf pov) because that call must return an IGatsbyNode whereas recursive calls can, but can also return a plain object. Not sure how to go about typing those tbh since it's, from TS's pov, untyped arbitrary content.
| @@ -20,15 +51,15 @@ const sanitizeNode = (data, isNode = true, path = new Set()) => { | |||
| returnData[key] = o | |||
| return | |||
| } | |||
| returnData[key] = sanitizeNode(o, false, path) | |||
| returnData[key] = sanitizeNode(o as T, false, path) | |||
There was a problem hiding this comment.
This may not be an IGatsbyNode, or even whatever Node is, as it's an arbitrary object inside a node so casting it is probably not what we want here?
|
Hi @arthurjdam , can you please confirm whether or not you'd like to continue working on this PR? Thank you 💜 |
pvdz
added
the
status: awaiting author response
|
Your pull request can be previewed in Gatsby Cloud: https://build-5048d338-6c45-42c0-8085-e6ff35954c8f.staging-previews.gtsb.io |
|
Looks like this is having some typechecking and linting issues after recent changes?. Mind making the updates and I'll prioritize getting this in? |
|
I'll close this PR as stale. Thanks for the PR though! |
Description
Migrates remaining classes in src/db to Typescript.
The export at the bottom of
src/db/nodes.tsis a little undesirable but the only way I could get it to work without rewriting all the other require()s.Open to suggestions!
Related Issues
Related to #21995