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

Generate TypeScript typings from Stone specification for NPM distribution #96

Merged
merged 1 commit into from
Mar 27, 2017

Conversation

jvilk
Copy link

@jvilk jvilk commented Jan 22, 2017

Future publications of the dropbox NPM module will now include TypeScript typings
generated using the new TypeScript stone generator. Typings contain TSDoc annotations
for better IDE integration.

This PR contains the following changes:

  • Modification to generate_routes.py to generate TypeScript typings from stone specifications.
  • Templates for TypeScript typings that include the needed boilerplate to expose routes under the appropriate module names.
    • These templates also include a few handwritten types / methods that this SDK introduces that are not part of a Stone specification (e.g., actAsUser, constructors, ...).
  • Modification to build scripts that appropriately includes typings into distributions.
  • Augments package.json with a pointer to the TypeScript definition file describing the entry module (src/index.d.ts).

The typings should work for people using Dropbox in a variety of settings:

  • From NPM as a commonjs module (via src/index.d.ts, which points to files in dist)
  • As a standalone library that exposes a global (via the .d.ts files in dist)
    • Generates definitions for both dropbox.js and dropboxteam.js

Fixes #65.

Please see the following Stone pull request for information on how you can test these typings yourself. I have also applied the typings to some SDK example code to verify that they work. :)

@jvilk
Copy link
Author

jvilk commented Jan 22, 2017

Let me know if you require additional modifications, such as the addition of some type of unit tests.

Unit testing would best be accomplished by compiling sample code against the generated typings. I have two ideas for this:

  • Convert your JS examples and unit tests into TypeScript, like I did in this repository for SDK examples.
    • The generated JavaScript should match your existing JavaScript exactly. (I can disable sourcemap generation to remove the sourcemap comment.)
    • Downside: You'll have to use TypeScript when you modify these in the future.
    • Upside: You only need to maintain a single set of files for testing both JS and TS, and you can point users to TypeScript and JavaScript example code.
  • Maintain a separate set of TypeScript examples.
    • Upside: You won't have to learn TypeScript.
    • Downside: If these examples become stale, then you will not know if the TypeScript typings are working for any new or modified features as the specification evolves.

@greg-db
Copy link
Contributor

greg-db commented Jan 23, 2017

Thanks for putting this together John!

@pran1990
Copy link
Contributor

pran1990 commented Feb 28, 2017

The generator works fine; I am working to make sure the examples @jvilk wrote are also in this repo.

For Unittests, I'm going to test the generated JS with our existing tests, as well as check that they match our existing JS exactly. I'll write TypeScript tests outside of this PR.

@jvilk
Copy link
Author

jvilk commented Mar 13, 2017

Let me know if you have any questions or need any help.

@pran1990
Copy link
Contributor

Hey @jvilk. First off, thanks a lot for the thorough work! A few requests when you can find the time.

Could you rebase this commit on top of master. I would like to merge this Pull request instead of doing it myself. I tested that things work (using your other example repo).

After rebasing, you might want to generate the bindings again to accommodate some routes we added (I can do that myself too; I kept you waiting for a while, so you might have lost context. You did add in the right changes needed to our generator scripts).

I would like to move the example repo into this one. I made a change to make the js examples live in a subfolder(examples/js), but started seeing issues when trying to run the ts examples when they live in a subfolder (instead of a separate repo). So I decided to come back to it later.

This branch is my attempt to do all of the above, it works in the trivial sense, ie, merges and builds correctly. I'll fix the examples-not-working problem, this PR doesn't have to wait for that.

@jvilk
Copy link
Author

jvilk commented Mar 16, 2017

Just successfully rebased. Going to update specifications, add in examples, and squash commits together next.

…tion

Future publications of the dropbox NPM module will now include TypeScript typings
generated using the new TypeScript stone generator. Typings contain TSDoc annotations
for better IDE integration.

Includes TypeScript versions of each JavaScript SDK example.
@jvilk
Copy link
Author

jvilk commented Mar 16, 2017

@pran1990 all done! To avoid cluttering up your package.json with TypeScript-related devDependencies, the examples/typescript folder has its own package.json. If desired, I can merge its contents into the main package.json.

I had to remove the rewrite rule in the express server, since it rewrote URLs like /basic/basic.js into /basic/index.html. I do not know why that rule was present, as the examples I tested seem to work flawlessly without it!

If the examples ever need to be changed, make sure you run npm run build in the examples/typescript folder after changing them to re-build the JavaScript files from the TypeScript source.

Let me know if you have any trouble with the examples. I encourage you to independently test them in case I missed something important.

@pran1990 pran1990 merged commit 02e4561 into dropbox:master Mar 27, 2017
@jvilk
Copy link
Author

jvilk commented Mar 28, 2017

🎉 thanks for merging!

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