Skip to content
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

TypeScript: Improve Definition #238

Open
6 tasks done
sgtoj opened this issue May 4, 2017 · 18 comments
Open
6 tasks done

TypeScript: Improve Definition #238

sgtoj opened this issue May 4, 2017 · 18 comments

Comments

@sgtoj
Copy link
Contributor

sgtoj commented May 4, 2017

List of improvements I believe is needed for the TypeScript definition. I have already started working on this list. Love to hear feedback if you have any.

  • Organize / Restructure
  • Add JSDoc for all functions
  • Define td.func()
  • Add generic types to td.function() and td.func()
  • Define td.constructor()
  • Add to ./test/src/typescript/test.ts to reflect any additions.

Changes: sgtoj/testdouble.js:improve-ts

@searls
Copy link
Member

searls commented May 4, 2017

Any improvements are appreciated. Hey @duluca, you'd suggested helping out with our typescript story, want to look at this?

@sgtoj
Copy link
Contributor Author

sgtoj commented May 4, 2017

Updated OP with the link to the fork and branch.

@sgtoj
Copy link
Contributor Author

sgtoj commented May 4, 2017

PR #239

@duluca
Copy link
Contributor

duluca commented May 4, 2017

Will review. @sgtoj please lmk if there's anything specific I can help with. I'll leave any comments re: code on the pr

@sgtoj
Copy link
Contributor Author

sgtoj commented May 4, 2017

Disclaimer: I am not the best when it comes to communication so there may be the grammar mistakes or typos. I did a quick proof reading before committing.

The jsdoc comments probably could use some additions or improvements from someone who has more experience with the framework and/or understanding behind the abstractions. Otherwise, it should be ready.

@duluca
Copy link
Contributor

duluca commented May 4, 2017

Sounds good. Will give it a shake tonight.

@albertogasparin
Copy link

Sorry to jump in, but with published typings I'm getting an error in VSCode (Typescript 2.3):

const myMock = td.object(['run', 'search']);
td.when(myMock.run()).thenCallback(null);
// Error: Property 'run' does not exist on type 'DoubledObjectWithKey<"run" | "search">'.

Is it something this PR is going to solve? Or are you guys not getting this?

@searls
Copy link
Member

searls commented May 9, 2017

@albertogasparin I suggest you attempt checking out the branch in #239 to find out for yourself

@albertogasparin
Copy link

Yep, was doing it. In fact, I'm still getting the same error even with the PR typings. Unfortunately I don't have much experience with Typescript, so I'm unable propose a fix

@sgtoj
Copy link
Contributor Author

sgtoj commented May 11, 2017

@albertogasparin I will look in to this issue. Hopefully, I will find a fix, add a test, and submit a new PR.

@sgtoj
Copy link
Contributor Author

sgtoj commented May 11, 2017

I fixed the issue and added a new TS test in #239. @albertogasparin @searls

Example

@albertogasparin
Copy link

@sgtoj Worked like a charm, thanks!
Hope to see #239 merged soon 😉

@searls
Copy link
Member

searls commented May 11, 2017

Good to hear. I'm chatting with @duluca tomorrow and want to have a chat with him about our TS approach more broadly before merging

@SimonSchick
Copy link

Any updates on this?

@sgtoj
Copy link
Contributor Author

sgtoj commented Sep 13, 2017

Let me know if there is anything I forgot to do or address.

@duluca
Copy link
Contributor

duluca commented Sep 13, 2017

@sgtoj as noted on PR #239, there are a couple of tasks due:
TODO:
[ ] Update from master, resolve conflicts
[ ] Update PR and make sure CI checks pass

@sgtoj
Copy link
Contributor Author

sgtoj commented Sep 15, 2017

@duluca I believe we are ready.

@duluca
Copy link
Contributor

duluca commented Sep 15, 2017

Close, but no cigar. The test code must live in the regression/test.ts file.

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

No branches or pull requests

5 participants