Skip to content

Conversation

@chris--young
Copy link
Contributor

Added a new response type for getAuth() since it should not use the same response type as getAccounts()

plaid.com/docs/api/#auth

> tsc

src/account/controller.ts:38:25 - error TS2339: Property 'numbers' does not exist on type 'AccountsResponse'.

38         console.log(res.numbers);
                           ~~~~~~~

Also moved types out of dev dependencies so that theyll be included with npm install --save plaid to prevent missing types

> tsc

node_modules/plaid/index.d.ts:2:31 - error TS7016: Could not find a declaration file for module 'request'. 'C:/Users/young/Desktop/book/node_modules/request/index.js' implicitly has an 'any' type.
  Try `npm install @types/request` if it exists or add a new declaration (.d.ts) file containing `declare module 'request';`

2   import { CoreOptions } from 'request';
                                ~~~~~~~~~

@chris--young
Copy link
Contributor Author

Fixes: #160

@diit
Copy link

diit commented Jul 18, 2018

We're having this issue as well, anything I can do to help get it merged in?

Copy link
Contributor

@ntindall ntindall left a 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;
Copy link
Contributor

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
Copy link
Contributor

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,
Copy link
Contributor

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",
Copy link
Contributor

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).

@ntindall
Copy link
Contributor

ntindall commented Aug 3, 2018

@diit, we will merge it in as soon as the comments are addressed. Sorry for the delay!
cc @michaelckelly @nathankot @pbernasconi

@chris--young
Copy link
Contributor Author

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

Copy link
Contributor

@ntindall ntindall left a 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/
Copy link
Contributor

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.

@ntindall ntindall merged commit b0856da into plaid:master Aug 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants