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

chore: add TypeScript typings file #926

Merged
merged 46 commits into from
Mar 26, 2019
Merged

Conversation

watson
Copy link
Contributor

@watson watson commented Mar 16, 2019

Closes #586

Checklist

  • Implement code
  • Fix TODO's
  • Add tests
  • Add TypeScript usage docs

Qard
Qard previously approved these changes Mar 16, 2019
@watson
Copy link
Contributor Author

watson commented Mar 17, 2019

I'm obviously doing something wrong with the shared function definitions. They have different icons in VS Code compared to the ones defined inline (e.g. addTags vs. addFilter):

image

Copy link
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

Overall LGTM :)
You need to add some testing as well, here and here you can find how I'm testing the types in the js client.

/cc @timroes, he helped me so much while I was writing the type definitions for the client :)

index.d.ts Show resolved Hide resolved
start.d.ts Outdated Show resolved Hide resolved
@watson
Copy link
Contributor Author

watson commented Mar 17, 2019

I asked on SO about how to fix the issue with the shared class methods: https://stackoverflow.com/questions/55208211/how-to-share-a-typescript-function-definition-between-two-classes

@watson
Copy link
Contributor Author

watson commented Mar 17, 2019

🤔 I just realized that this "feature" makes .d.ts files a bit annoying for users: microsoft/vscode#68782

index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
bmorelli25
bmorelli25 previously approved these changes Mar 25, 2019
Copy link
Member

@bmorelli25 bmorelli25 left a comment

Choose a reason for hiding this comment

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

Docs LGTM

@watson watson dismissed stale reviews from bmorelli25 and vigneshshanmugam via a4a1e4c March 25, 2019 16:44
@watson
Copy link
Contributor Author

watson commented Mar 25, 2019

testing some hooks... please ignore 😎

Qard
Qard previously approved these changes Mar 25, 2019
@nik9000
Copy link
Member

nik9000 commented Mar 25, 2019

I'm looking at the docs-build failure. I believe it is safe to ignore but I'll track it down!

@nik9000
Copy link
Member

nik9000 commented Mar 25, 2019

I'm looking at the docs-build failure. I believe it is safe to ignore but I'll track it down!

I've tracked down the issue and will submit a PR to fix it to the infra repo. Sorry for the trouble. Thanks for being my first victims!

The removed versions all depend on apollo-upload-server@5.0.0 which is
broken because of a missing dependency: core-js

The reason why it worked before was because one of our other
devDependencies had the correct core-js dependency. With the added
dev-dependency for Babel, this core-js dependency have now been updated
to a version unsupported by apollo-upload-server, and so the tests for
apollo-server-express <2.0.5 now fails.
@nik9000
Copy link
Member

nik9000 commented Mar 25, 2019

@elasticmachcine, run docs build.

@nik9000
Copy link
Member

nik9000 commented Mar 25, 2019

@elasticmachine, run docs build.

@nik9000
Copy link
Member

nik9000 commented Mar 25, 2019

I'm looking at the docs-build failure. I believe it is safe to ignore but I'll track it down!

I've tracked down the issue and will submit a PR to fix it to the infra repo. Sorry for the trouble. Thanks for being my first victims!

Fixed!

@watson
Copy link
Contributor Author

watson commented Mar 25, 2019

jenkins run tests please

@watson watson requested a review from Qard March 25, 2019 22:05
@spalger
Copy link

spalger commented Apr 23, 2019

Thank you!!! 🌮

@smhmayboudi
Copy link

I think it is better to use the npm package definitions instead of writing them.

import AwsLambda from "aws-lambda";
import Connect from "connect";

// Inlined from @types/aws-lambda - start

// Inlined from @types/connect - start

@smhmayboudi
Copy link

I copy and paste this file to my project. Is there another way to reach the interfaces and class definitions?

@smhmayboudi
Copy link

I read the docs and find out logUncaughtExceptions is not defined in AgentConfigOptions. Would you please fix it.

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.

Add @types for typescript
10 participants