fix(typescript): fix query typings #32

manakuro · 2019-06-13T03:48:23Z

What:
Fix Query typings.

Why:
Typings error occurred because of incompatible with Query interface as noted in #30.

How:

Use NativeTestInstance type for returning Query interface.
Remove NativeTestInstance type from testRenderer argument in Query interface.
Fix ReactTestInstance.getProps type.

Checklist:

Documentation added to the
docs site
Typescript definitions updated
Tests
Ready to be merged

Results:

$ yarn tsc                                                                                                                                                                          
yarn run v1.15.2
$ /Users/me/workspace/react-native/react-native-sample/node_modules/.bin/tsc
✨  Done in 30.35s.

* fix(typescript): fix query typings * fix(typescript): remove NativeTestInstance from function argument in Query interface * chore: remove unnecessary code

codecov-io · 2019-06-13T03:52:05Z

Codecov Report

Merging #32 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master    #32   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          20     20           
  Lines         262    262           
  Branches       65     65           
=====================================
  Hits          262    262

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 204ff08...ec81517. Read the comment docs.

manakuro · 2019-06-13T03:52:43Z

@bcarroll22 I'm sorry for the late reply. I would love to get feedback on this!

bcarroll22

This is awesome, thanks for tackling this! If you could please address those few things I mentioned we’ll get this merged ASAP!

bcarroll22 · 2019-06-13T12:25:49Z

typings/index.d.ts

@@ -10,12 +10,12 @@ import { getQueriesForElement, BoundFunction } from './get-queries-for-element';
 declare const within: typeof getQueriesForElement;

 interface Query extends Function {
-  (testRenderer: ReactTestRenderer | NativeTestInstance, ...args: any[]):
+  (testRenderer: ReactTestRenderer, ...args: any[]):


I think this may be backwards. A query only takes a NativeTestInstance, never a ReactTestRenderer.

Oh I just thought testRenderer should be ReactTestRenderer by seeing these changes.

manakuro@d8c3b30#diff-0da64c26f363af99c40bbae0dc3fb0bfR8

Should it be replaced with NativeTestInstance in typings/queries.d.ts like this ?

export type QueryByBoundProp = ( testRenderer: NativeTestInstance, id: Matcher, options?: MatcherOptions, ) => NativeTestInstance | null; ...

and Query just takes NativeTestInstance ?

interface Query extends Function { (testRenderer: NativeTestInstance, ...args: any[]): | Error | Promise<NativeTestInstance[]> | Promise<NativeTestInstance> | NativeTestInstance[] | NativeTestInstance | null; }

Yep this is right. And to make it more clear, we might want to change the name of the argument from testRenderer as a part of this as well 👍

bcarroll22 · 2019-06-13T12:26:18Z

typings/query-helpers.d.ts

@@ -4,12 +4,12 @@ import { Matcher, MatcherOptions } from './matches';

 type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>>;

-export interface SelectorMatcherOptions extends MatcherOptions {
+export type SelectorMatcherOptions = Omit<MatcherOptions, 'selector'> & {


Can you help me understand what this one does?

The previous one tried to extend MatcherOptions and override selector property but TypeScript cannot override interface property. So to do that, you can use Omit to exclude selector and extend it again.

https://stackoverflow.com/questions/41285211/overriding-interface-property-type-defined-in-typescript-d-ts-file?answertab=votes#tab-top

Got it, thanks 👍

bcarroll22 · 2019-06-13T12:27:21Z

typings/query-helpers.d.ts


 type ReactTestInstance = {
-  getProp: (string) => NativeTestInstance;
+  getProp: (str: string) => NativeTestInstance;


Can you make this something more descriptive of what this is? Like name possibly?

Yes. This is just a syntax error. You need to use type and name together.

https://www.typescriptlang.org/docs/handbook/functions.html#writing-the-function-type

Yep, can we make the name of the argument name so that it’s clear from the typings what getProp is looking for? ☺️

mAAdhaTTah · 2019-06-13T20:44:29Z

This should also replace asFragment with asJSON, which is incorrect in the typings.

bcarroll22 · 2019-06-17T12:09:50Z

@mAAdhaTTah in case this PR doesn’t address your concern or you need it faster, I’ll review one from you today that fixes it if you’d like?

mAAdhaTTah · 2019-06-19T16:15:42Z

@bcarroll22 PR'd as #34 but we're going to need both of these PRs to get TS to stop complaining.

bcarroll22 · 2019-06-20T01:28:33Z

@manakuro any ideas when we can wrap this one up?

bcarroll22 · 2019-06-20T18:13:20Z

I'm going to go ahead and get this merged and make the few changes so that this change is working for others as well. Thanks for the help!

@allcontributors please add @manakuro for code.

allcontributors · 2019-06-20T18:13:30Z

@bcarroll22

I've put up a pull request to add @manakuro! 🎉

bcarroll22 · 2019-06-20T18:24:56Z

🎉 This PR is included in version 4.0.3 🎉

The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot 📦🚀

fix(typescript): fix query typings (#1)

e33f28f

* fix(typescript): fix query typings * fix(typescript): remove NativeTestInstance from function argument in Query interface * chore: remove unnecessary code

bcarroll22 suggested changes Jun 13, 2019

View reviewed changes

bcarroll22 mentioned this pull request Jun 19, 2019

Typescript Error: findAllByTestId #35

Closed

Merge branch 'master' into master

ec81517

allcontributors bot mentioned this pull request Jun 20, 2019

docs: add manakuro as a contributor #37

Merged

bcarroll22 merged commit c6088ad into testing-library:master Jun 20, 2019

bcarroll22 added the released label Jun 20, 2019

Uh oh!

fix(typescript): fix query typings #32

fix(typescript): fix query typings #32

Uh oh!

Conversation

manakuro commented Jun 13, 2019

Uh oh!

codecov-io commented Jun 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

manakuro commented Jun 13, 2019

Uh oh!

bcarroll22 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mAAdhaTTah commented Jun 13, 2019

Uh oh!

bcarroll22 commented Jun 17, 2019

Uh oh!

mAAdhaTTah commented Jun 19, 2019

Uh oh!

bcarroll22 commented Jun 20, 2019

Uh oh!

bcarroll22 commented Jun 20, 2019

Uh oh!

allcontributors bot commented Jun 20, 2019

Uh oh!

bcarroll22 commented Jun 20, 2019

Uh oh!

Uh oh!

codecov-io commented Jun 13, 2019 •

edited

Loading