refreshToken and types #23
Conversation
Per documentation at: https://developer.tdameritrade.com/price-history/apis/get/marketdata/%7Bsymbol%7D/pricehistory The values for frequencyType should be: 'minute' | 'daily' | 'weekly' | 'monthly'
Remove 'access_type' from URLSearchParams
There was a problem hiding this comment.
Again thank you for taking the time to contribute. Unfortunately, I can't accept the PR as is because it:
- Leaves failing tests behind (
npm tests) and must also be changed. - Cripples
td.login()which is expected to renew the refresh token as well.
Perhaps a better approach would be to add a way to renew the access and refresh tokens separately OR even better with the same method only renew the access token and renew both once the refresh token reaches close to its expiration day. Though to be fair, I am not convinced it'll add much value. What do you think?
| @@ -955,7 +955,7 @@ export type PriceHistoryQuery = { | |||
| * - `year` : daily, weekly, monthly* | |||
| * - `ytd` : daily, weekly* | |||
| */ | |||
| frequencyType: 'day'|'month'|'year'|'ytd'; | |||
| frequencyType: 'minute'|'daily'|'weekly'|'monthly'; | |||
There was a problem hiding this comment.
The types should be changed in the jsdoc comments rather than the type declarations file because the declarations are generated using the npm types script. See package.json and the script. Also, I wouldn't mind this being a separate PR.
make start/end dates optional
|
Baby steps! Thank you for the feedback... I'll make the changes you identified and re-submit. |
resolves knicola/tdameritradejs #22