Added new types for trouter #33557

demaggus83 · 2019-03-03T08:31:45Z

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change. (Run with npm test.)
Follow the advice from the readme.
Avoid common mistakes.
Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If adding a new definition:

The package does not already provide its own types, or cannot have its .d.ts files generated via --declaration
If this is for an NPM package, match the name. If not, do not conflict with the name of an NPM package.
Create it with dts-gen --dt, not by basing it on an existing project.
tslint.json should be present, and tsconfig.json should have noImplicitAny, noImplicitThis, strictNullChecks, and strictFunctionTypes set to true.

typescript-bot · 2019-03-03T09:22:43Z

@stahlstift Thank you for submitting this PR!

Because this is a new definition, a DefinitelyTyped maintainer will be reviewing this PR in the next few days once the Travis CI build passes.

In the meantime, if the build fails or a merge conflict occurs, I'll let you know. Have a nice day!

types/trouter/index.d.ts

typescript-bot · 2019-03-03T21:22:43Z

@stahlstift One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits or comments. Thank you!

types/trouter/index.d.ts

lukeed · 2019-03-04T05:45:01Z

types/trouter/index.d.ts

+
+    add(method: Trouter.ValidMethods, pattern: string, ...handlers: T[]): this;
+
+    all(pattern: string, ...handlers: T[]): this;


Not important, but I feel like there ought to be a way to move all of these into a shared definition... possibly extending add?

Something like this but for classes, of course.

@SimonSchick
I don't know if that is possible.
Do you know?

I don't think so given the return type is this.

types/trouter/trouter-tests.ts

typescript-bot · 2019-03-04T17:25:14Z

🔔 @SimonSchick - Thanks for your review of this PR! Can you please look at the new code and update your review status if appropriate?

lukeed

I included all HTTP methods, changing the type's name.
Also brought the definitions up to trouter@3.0.0 which was released over the weekend.

lukeed · 2019-03-11T18:13:15Z

PS – I know everything here is automated, but if it helps fast-track things, I'm the library author 👍

types/trouter/index.d.ts

Co-Authored-By: lukeed <luke.edwards05@gmail.com>

types/trouter/trouter-tests.ts

Co-Authored-By: lukeed <luke.edwards05@gmail.com>

weswigham · 2019-03-11T20:55:51Z

types/trouter/index.d.ts

+// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
+// TypeScript Version: 2.3
+
+export interface FindResult<T> {


You want to put FindResult, HTTPMethod, and FindReturn into a namespace Trouter that merges with the class.

I've no idea what this means, but it was done originally with contention: e83a760#r261887776

It was thought unneeded because of the incorrect default export - with that gone, since it's now using export = to match runtime behavior, the namespace is actually required.

Okay, thanks for the explanation 👍 I'll wrap it as before. However, does this mean that users won't be able to import the HTTPMethod & co?

You'll still be able to. import {HTTPMethod} from "trouter" will work fine - you'll just need to import Trouter = require("trouter") for the class. Trouter.HTTPMethod will also work on the latter.

Great, thanks for your help and patience :)

Sorry about this, I've been reviewing quite a lot of packages the last couple of days and might've mixed up exports in this case.

All good! I was confused to begin with 😉

typescript-bot · 2019-03-11T21:30:36Z

@stahlstift The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

- part of 3.0 release

typescript-bot · 2019-03-11T22:28:21Z

I just published @types/trouter@3.0.0 to npm.

Added types for trouter

e83a760

demaggus83 mentioned this pull request Mar 3, 2019

TypeScript typings support? lukeed/polka#17

Open

typescript-bot added this to Review in Pull Request Status Board Mar 3, 2019

typescript-bot added New Definition This PR creates a new definition package. Awaiting reviewer feedback labels Mar 3, 2019

SimonSchick suggested changes Mar 3, 2019

View reviewed changes

types/trouter/index.d.ts Outdated Show resolved Hide resolved

types/trouter/index.d.ts Show resolved Hide resolved

typescript-bot moved this from Review to Needs Author Attention in Pull Request Status Board Mar 3, 2019

typescript-bot added Revision needed This PR needs code changes before it can be merged. and removed Awaiting reviewer feedback labels Mar 3, 2019

lukeed reviewed Mar 4, 2019

View reviewed changes

Fixes

7545bf5

lukeed added 2 commits March 11, 2019 11:01

fix: include all valid HTTP methods

0ac4aaa

feat: add use(); bump to 3.0

7534d5b

lukeed approved these changes Mar 11, 2019

View reviewed changes

SimonSchick approved these changes Mar 11, 2019

View reviewed changes

typescript-bot moved this from Needs Author Attention to Review in Pull Request Status Board Mar 11, 2019

typescript-bot added Other Approved This PR was reviewed and signed-off by a community member. Awaiting reviewer feedback labels Mar 11, 2019

weswigham requested changes Mar 11, 2019

View reviewed changes

types/trouter/index.d.ts Outdated Show resolved Hide resolved

typescript-bot moved this from Review to Needs Author Attention in Pull Request Status Board Mar 11, 2019

typescript-bot removed Other Approved This PR was reviewed and signed-off by a community member. Awaiting reviewer feedback labels Mar 11, 2019

weswigham and others added 2 commits March 11, 2019 13:36

fix: change export

ee2e836

Co-Authored-By: lukeed <luke.edwards05@gmail.com>

fix: use require in test

04e34c0

weswigham reviewed Mar 11, 2019

View reviewed changes

types/trouter/trouter-tests.ts Outdated Show resolved Hide resolved

fix: incorrect import statement

8928d02

Co-Authored-By: lukeed <luke.edwards05@gmail.com>

weswigham reviewed Mar 11, 2019

View reviewed changes

typescript-bot added the The Travis CI build failed label Mar 11, 2019

lukeed added 2 commits March 11, 2019 15:06

fix: use namespace

2e6df45

fix: remove false return option;

4a19bbc

- part of 3.0 release

weswigham merged commit 08257fb into DefinitelyTyped:master Mar 11, 2019

Pull Request Status Board automation moved this from Needs Author Attention to Done Mar 11, 2019

typescript-bot removed this from Done in Pull Request Status Board Mar 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added new types for trouter #33557

Added new types for trouter #33557

demaggus83 commented Mar 3, 2019

typescript-bot commented Mar 3, 2019 •

edited

typescript-bot commented Mar 3, 2019

lukeed Mar 4, 2019

demaggus83 Mar 4, 2019

SimonSchick Mar 4, 2019

typescript-bot commented Mar 4, 2019

lukeed left a comment

lukeed commented Mar 11, 2019

weswigham Mar 11, 2019

lukeed Mar 11, 2019

weswigham Mar 11, 2019 •

edited

lukeed Mar 11, 2019

weswigham Mar 11, 2019

lukeed Mar 11, 2019

SimonSchick Mar 11, 2019

lukeed Mar 11, 2019

typescript-bot commented Mar 11, 2019

typescript-bot commented Mar 11, 2019


		add(method: Trouter.ValidMethods, pattern: string, ...handlers: T[]): this;

		all(pattern: string, ...handlers: T[]): this;

Added new types for trouter #33557

Added new types for trouter #33557

Conversation

demaggus83 commented Mar 3, 2019

typescript-bot commented Mar 3, 2019 • edited

typescript-bot commented Mar 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

typescript-bot commented Mar 4, 2019

lukeed left a comment

Choose a reason for hiding this comment

lukeed commented Mar 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weswigham Mar 11, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

typescript-bot commented Mar 11, 2019

typescript-bot commented Mar 11, 2019

typescript-bot commented Mar 3, 2019 •

edited

weswigham Mar 11, 2019 •

edited