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

Add Typescript typechecking, generate types with tsc #153

Merged
merged 5 commits into from
Aug 22, 2022

Conversation

nrabinowitz
Copy link
Collaborator

  • Adds type-checking via tsc --noEmit
  • Updates types in JSDoc comments to fix TS errors
  • Drops tsd-jsdoc in favor of generating the library types with tsc --declaration
  • Adds types for CoordPair and SplitLong, post-processing the libdef to use [number, number] instead of number[] (this is a workaround for Support Tuples jsdoc/jsdoc#1703, which kills doc generation if we use [number, number] directly in JSDoc comments)

Fixes #152

@isaacbrodsky
Copy link
Collaborator

Can this be merged after v4.0.0 is released in both core library & JS?

@nrabinowitz
Copy link
Collaborator Author

Can this be merged after v4.0.0 is released in both core library & JS?

Technically changing types could be considered breaking for TS users, so I'd prefer to land this pre-release.

Nick Rabinowitz added 2 commits August 22, 2022 09:43
@coveralls
Copy link

coveralls commented Aug 22, 2022

Pull Request Test Coverage Report for Build 2907065074

  • 22 of 22 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 99.844%

Totals Coverage Status
Change from base Build 2878765090: -0.2%
Covered Lines: 529
Relevant Lines: 529

💛 - Coveralls

package.json Outdated
@@ -30,7 +30,8 @@
"build-emscripten": "yarn build-update-h3 && yarn docker-emscripten",
"build-legacy": "node scripts/build-legacy-api.js",
"build-docs": "jsdoc2md --no-cache --global-index-format grouped --partial doc-files/scope.hbs --helper ./doc-files/insert-version.js --separators --template doc-files/README.tmpl.md lib/h3core.js lib/errors.js > README.md",
"build-tsd-core": "jsdoc -t node_modules/tsd-jsdoc/dist -d console lib/h3core.js | sed 's/\"h3\"/\"h3-js\"/g' > dist/types.d.ts",
"build-tsd-core": "tsc lib/h3core.js --noResolve --skipLibCheck --allowJs --declaration --emitDeclarationOnly --outFile dist/types.d.ts && yarn build-tsd-core-postprocess",
"build-tsd-core-postprocess": "sed 's/module \"h3core\"/module \"h3-js\"/g' dist/types.d.ts > types.d.ts.tmp && sed -E -e 's/(CoordPair|SplitLong) = number\\[\\]/\\1 = [number,number]/g' types.d.ts.tmp > dist/types.d.ts && rm types.d.ts.tmp",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this logic should be in a script file so that comments can be added to it? This is a little 'magical'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, I can take a look at moving this into a script

@nrabinowitz nrabinowitz merged commit ece84ef into uber:master Aug 22, 2022
@nrabinowitz nrabinowitz deleted the typescript-libdef branch August 22, 2022 21:15
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.

Documentation of the [lat, lng] tuple
3 participants