-
Notifications
You must be signed in to change notification settings - Fork 177
Fix /auth/get response types #162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Fixes: #160 |
|
We're having this issue as well, anything I can do to help get it merged in? |
ntindall
left a comment
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.
@chris--young left a few comments - great catch and thank you for working on this.
index.d.ts
Outdated
|
|
||
| interface BaseResponse { | ||
| request_id: string; | ||
| status_code: number; |
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 don't believe status_code is returned in the base response
index.d.ts
Outdated
| account: string, | ||
| account_id: string, | ||
| routing: string, | ||
| write_routing: string |
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.
from the documentation for Auth see here this should be wire_routing
index.d.ts
Outdated
| } | ||
|
|
||
| interface ACHNumbers { | ||
| account: string, |
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.
nit: can we use ; as the trailing delimiter on the typings (as we do elsewhere in the file)?
| "bluebird": "^3.5.1", | ||
| "ramda": "0.23.x", | ||
| "request": "^2.74.0" | ||
| "request": "^2.74.0", |
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 believe this is the correct thing for us to do, given that we are exporting the typings from the package (for typescript clients, it will matter for those that do not install devDependencies).
|
@diit, we will merge it in as soon as the comments are addressed. Sorry for the delay! |
|
no worries about the delay, just made a few changes to address these comments also been having a hard time running getting this make file to work on windows might be willing to open up and issue and fix that if you think its worth it |
ntindall
left a comment
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.
LGTM
| /node_modules/ | ||
| /npm-debug.log | ||
| .env | ||
| .vs/ |
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.
you probably need to add this to your .gitignore_global file @chris--young
I don't mind it being comitted here as well, though.
Added a new response type for getAuth() since it should not use the same response type as getAccounts()
plaid.com/docs/api/#auth
Also moved types out of dev dependencies so that theyll be included with
npm install --save plaidto prevent missing types