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

Update types for fetch-mock v5.8.1 #13784

Merged
1 commit merged into from Jan 9, 2017
Merged

Conversation

merrywhether
Copy link
Contributor

@merrywhether merrywhether commented Jan 6, 2017

Please fill in this template.

  • Make your PR against the master branch.
  • 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.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run tsc without errors.
  • Run npm run lint package-name if a tslint.json is present.

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: <https://github.com/wheresrhys/fetch-mock> (no change log in repo sadly)
  • Increase the version number in the header if appropriate.
  • If you are making substantial changes, consider adding a tslint.json containing tslint.

Fixed some lint errors (uses of Object, missing semicolons, and collapsable overloads) and added once(), done(), catch(), and spy().

@dt-bot
Copy link
Member

dt-bot commented Jan 6, 2017

fetch-mock/index.d.ts

to authors (@asvetliakov @tamird). Could you review this PR?
👍 or 👎?

Checklist

  • pass the Travis CI test?

Copy link
Contributor

@tamird tamird left a comment

Choose a reason for hiding this comment

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

Minor comments, but LGTM.

@@ -69,7 +69,7 @@ interface MockResponseObject {
type MockResponse = Response | Promise<Response>
| number | Promise<number>
| string | Promise<string>
| Object | Promise<Object>
| {} | Promise<{}>
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the upshot of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was based on a suggestion from tslint:
forbidden-types - Avoid using the Object type. Did you mean 'any' or '{}'?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, OK.

}

type MockCall = [string, MockRequest];

interface MatchedRoutes {
matched: Array<MockCall>;
unmatched: Array<MockCall>;
matched: MockCall[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Again; why is this better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tslint again:
array-type - Array type using 'Array' is forbidden for simple types. Use 'T[]' instead.`

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, OK.

call through to fetch(). Calls to .mock() can be chained.
* @param options The route to mock
call through to fetch(). Shorthand for mock() limited to being
called one time only. Calls to .mock() can be chained.
Copy link
Contributor

Choose a reason for hiding this comment

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

should be "calls to .once()"

@@ -171,6 +183,18 @@ interface FetchMockStatic {
/**
* Replaces fetch() with a stub which records its calls, grouped by
route, and optionally returns a mocked Response object or passes the
call through to fetch(). Shorthand for mock() restricted to the GET
method and limited to being called one time only. Calls to .mock()
Copy link
Contributor

Choose a reason for hiding this comment

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

s/mock/getOnce/

@@ -181,6 +205,18 @@ interface FetchMockStatic {
/**
* Replaces fetch() with a stub which records its calls, grouped by
route, and optionally returns a mocked Response object or passes the
call through to fetch(). Shorthand for mock() restricted to the POST
method and limited to being called one time only. Calls to .mock()
Copy link
Contributor

Choose a reason for hiding this comment

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

s/mock/postOnce/

* Replaces fetch() with a stub which records its calls, grouped by
route, and optionally returns a mocked Response object or passes the
call through to fetch(). Shorthand for mock() restricted to the HEAD
method and limited to being called one time only. Calls to .mock()
Copy link
Contributor

Choose a reason for hiding this comment

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

s/mock/headOnce/

* Replaces fetch() with a stub which records its calls, grouped by
route, and optionally returns a mocked Response object or passes the
call through to fetch(). Shorthand for mock() restricted to the PATCH
method. Calls to .mock() can be chained.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/mock/patch/

* Replaces fetch() with a stub which records its calls, grouped by
route, and optionally returns a mocked Response object or passes the
call through to fetch(). Shorthand for mock() restricted to the PATCH
method and limited to being called one time only. Calls to .mock()
Copy link
Contributor

Choose a reason for hiding this comment

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

s/mock/patchOnce/

catch(response?: MockResponse | MockResponseFunction): this;

/**
* Chainable method this records the call history of unmatched calls,
Copy link
Contributor

Choose a reason for hiding this comment

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

s/this/that/

was matched.
*/
called(): boolean;
calls(matcherName?: string): MockCall[];
Copy link
Contributor

Choose a reason for hiding this comment

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

again, why Array<MockCall>->MockCall[]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tslint again

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, OK.

@merrywhether
Copy link
Contributor Author

Made suggested docstring changes

@tamird
Copy link
Contributor

tamird commented Jan 9, 2017

👍

@ghost ghost merged commit c779608 into DefinitelyTyped:master Jan 9, 2017
@merrywhether merrywhether deleted the fetch-mock branch January 9, 2017 19:19
ericanderson added a commit to ericanderson/DefinitelyTyped that referenced this pull request Jan 17, 2017
* ericanderson/patch-1: (211 commits)
  [graphql] Meet types-2.0 style (DefinitelyTyped#13831)
  Update types for cookies v0.6.2 (DefinitelyTyped#13795)
  fixing incorrect version header, this should resolve the current npm version issue (DefinitelyTyped#13834) (DefinitelyTyped#13921)
  Fix tsconfig for node v6 (DefinitelyTyped#13924)
  Node: add timeout member to http.RequestOptions interface (DefinitelyTyped#13865)
  readme: Clarify how to add oneself as an author (DefinitelyTyped#13917)
  Xrm.Page.ui.process was pointing to Xrm.Page.data.ProcessManager, rather than Xrm.Page.ui.ProcessManager. (DefinitelyTyped#13920)
  scanf provides its own types (DefinitelyTyped#13919)
  leaflet-label: Use external module syntax (DefinitelyTyped#13918)
  Added module definition (DefinitelyTyped#13870)
  Update config to support adal 1.0.13 settings (DefinitelyTyped#13871)
  Adding geolib definition with fixed signatures in file taken from Vladimir Venegas. Tests are also added. (DefinitelyTyped#13886)
  Add missing 'allowLogin' parameter to show() options. Alphabetize show() options in type definition.
  tar: Add "noProprietary" header
  Fix argument list for options object to 'show()' function
  Update expr-eval for new linter
  Fix redux-form typings (DefinitelyTyped#13867)
  Added dir to JSZipFileOptions.
  Update index.d.ts
  Update index.d.ts
  Create tslint.json
  Address tslint issues
  Update auth0-lock to v10.9.1 API
  Make version number parse-able (DefinitelyTyped#13910)
  Added missing methods to Body.
  Add `closeOnClick` to PopupOptions
  Make version parse-able. (DefinitelyTyped#13904)
  Fix path mappings (DefinitelyTyped#13902)
  Remove MapStore for flux v3.0 (DefinitelyTyped#13819)
  Rename test files to be consistent (DefinitelyTyped#13882)
  Update comment on TS2.1 (DefinitelyTyped#13883)
  chart.js: Fix spelling of "afterFooter"
  Linting
  Remove unnecessary public identifiers
  initial commit
  Initial commit
  Add 'extensions' option.
  Fix version
  react-event-listener definitions
  Fixed dependent module
  Bump version
  Move CSSTransitionGroupProps to react module
  [pem] update typing info for pem 1.9.4
  Make mocha-node unused to prevent installing @types/node for browser users (DefinitelyTyped#13869)
  Update types for fetch-mock v5.8.1 (DefinitelyTyped#13784)
  Remove const enum label from node (now works with --isolatedModules)
  Update tsconfig.json
  Update index.d.ts
  Edits JSDoc
  Update sha1-tests.ts
  ...
This pull request was closed.
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.

None yet

3 participants