-
Notifications
You must be signed in to change notification settings - Fork 10
Enhance skygear cloud type definition #9
base: master
Are you sure you want to change the base?
Conversation
body?: string; | ||
} | ||
|
||
interface ResultJSON { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unused in this commit.
index.d.ts
Outdated
|
||
toResultJSON(): { [key: string]: any }; | ||
} | ||
namespace SkygearResponse { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a fan of namespace
so I prefer interface SkygearResponseOptions
@@ -489,6 +489,9 @@ declare module "skygear" { | |||
} | |||
|
|||
declare module "skygear/cloud" { | |||
import { Fields, Files } from "formidable"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the definition is small enough that we can copy instead.
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/formidable/index.d.ts#L32
I would like to reduce the number of dependencies.
], | ||
"dependencies": { | ||
"@types/formidable": "*", | ||
"@types/node": "*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can avoid @types/formidable
but not @types/node
. Maybe we should mark @types/node
as peer dependency and add it to devDependencies
so that we still can reference it in our test.
I can feel the needs to add ci that tests the definition in a server tsconfig.