This repository was archived by the owner on Jan 9, 2023. It is now read-only.
feat: Types for TypeScript included (partially) #94
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
activescott
commented
Jun 30, 2020
| "underscore": "^1.8.2", | ||
| "winston": "^2.3.1" | ||
| }, | ||
| "devDependencies": { |
Contributor
Author
There was a problem hiding this comment.
NOTE: I did add some dependencies here. They're all dev-only, but good to review.
|
Please, merge these changes. They're very important. |
Contributor
Author
|
@timwellswa Let me know if you want more done to merge this. I'm happy to do the necessary edits and testing to implement my suggestion #5 above if that helps. |
I had a weird local config that wasn't updating it
Will try this to see if it fixes and consider a fix to include running tests on old v8 node after this is confirmed
Should put this back using a githook instead
…tions & responses
activescott
force-pushed
the
feat/typescript-types
branch
from
c0babc4 to
9df2723
Compare
* tests for behavior and types of getSheet queryParameters option * adds a watch script to run tests * updates test and coverage scripts to be consistent w/ watch * adds requestor option to ClientOptions type
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've been using the SDK in TypeScript and I don't want to copy/paste type files around so I thought I'd try to get them into the SDK itself.
NOTE: If anyone is interested in trying these types out you can use
npm add git://github.com/activescott/smartsheet-javascript-sdk.git#feat/typescript-typesto use the code in this PR directly. I would love some feedback from others to help improve this or a comment indicating it worked for you would be great!Some notes for reviewers off the type of my head:
import smartsheet from "smartsheet"works now (in JS and TS). The oldimport * as smartsheet from "smartsheet"andrequiresyntax should continue to work (I added tests in TS)..tsfiles and embed the types in there. For example, instead of adding a newlib/types/sheets/sheets.d.ts, I could just convert/lib/sheets/index.jstolib/sheets/index.tsand embed the types in there. An example of that is already done in this PR by converting/index.jsto/index.ts.I recommend a squash merge commit.
Testing
test/types/index.spec.tsand are run withnpm testscript.npm packlocally and then installing from the tar withnpm i ~/src/smartsheet-javascript-sdk/smartsheet-2.86.0.tgzand it worked in my local typescript project.