chore(gatsby): Migrate utils/get-latest-apis.js to ts #22097
Conversation
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable @typescript-eslint/no-var-requires */ | |||
There was a problem hiding this comment.
I want to use require in order mock objects to be inferred as any. I came up with using as jest.Mock<> as an alternative but I guess this is a bit redundant and no different from using any.
| try { | ||
| const { data } = await axios.get(API_FILE, { timeout: 5000 }) | ||
|
|
||
| await fs.writeFile(OUTPUT_FILE, JSON.stringify(data, null, 2), `utf8`) | ||
|
|
||
| return data | ||
| } catch (e) { | ||
| if (await fs.exists(OUTPUT_FILE)) { | ||
| if (await fs.existsSync(OUTPUT_FILE)) { |
There was a problem hiding this comment.
I think we should use a sync method.
There was a problem hiding this comment.
How come? I don't see the benefit, but happy to be convinced!
There was a problem hiding this comment.
We await for the async one, so we could use the sync one instead.
But need to delete the await key then
There was a problem hiding this comment.
Still want to understand why we want the sync one. This does put a different expense on the system. When you await things, you allow the node process to handle other things on the stack that are paused. I'm not seeing the benefit of doing sync here.
There was a problem hiding this comment.
Sorry, when I search for fs-extra I found out that fs.exist(OUTPUT_FILE) returns a promise. So I shouldn't change to existSync. 🙇
fs-extrais a drop in replacement for nativefs. All methods in fs are attached tofs-extra. All fs methods return promises if the callback isn't passed.
ref
However, this is a bit troublesome because TS infers fs.exists as native fs.exists. The error An argument for 'callback' was not provided. still remains.
function exists(path: PathLike, callback: (exists: boolean) => void): void;And be careful that exist() of native fs returns void, not a promise and it needs a callback function on second argument. And this is deprecated.
There was a problem hiding this comment.
looks like we just want this instead: https://github.com/jprichardson/node-fs-extra/blob/master/docs/pathExists.md
if this doesn't work, I think the best thing would be to submit a PR to update the types in DefinitelyTyped
| export interface IAPIResponse { | ||
| [key: string]: any | ||
| } | ||
|
|
||
| export const getLatestAPIs = async (): Promise<IAPIResponse> => { |
There was a problem hiding this comment.
We need to define the interface of API response but this was too difficult for me. plz help 🙏
There was a problem hiding this comment.
it looks like it's some object with { browser: {}, node: {}, ssr: {} }, I would probably just add a console.log to data here and run the code so you can see what it prints out.
I think this gets triggered by having a gatsby plugin with exports that are not part of the gatsby spec.
There was a problem hiding this comment.
It has so many properties but I determined below for now.
interface IAPIResponse {
ssr: {
[key: string]: object
}
browser: {
[key: string]: object
}
node: {
[key: string]: object
}
}There was a problem hiding this comment.
i'd prefer to be explicit. Let's properly type it please!
Co-Authored-By: Michaël De Boey <[email protected]>
blainekasten
added
the
bot: merge on green
Description
Convert
utils/get-latest-apisto typescriptDocumentation
Related Issues
Related to #21995