Setup TS tests

[Andarist]

I've added infrastructure to test TS typings by possible use cases which should prevent regressions in the future (once more tests get added ofc)

[erikras]

This pull request introduces 2 alerts when merging 90a471e into 65a239b - view on LGTM.com

new alerts:

• 2 for Unused variable, import, function or class

---

Comment posted by LGTM.com

[Andarist]

@erikras LGTM report is actually an expected result (those rules should be disabled for .ts files as their purpose is to test interfaces and not to actually be executed), but LGTM setting is not in this repo (I think) and thus I cannot fix it :<

As to Travis - this is currently blocked by microsoft/dtslint#190

[codecov]

Codecov Report

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

@@          Coverage Diff          @@
##           master   #418   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          10     10           
  Lines         207    207           
  Branches       61     61           
=====================================
  Hits          207    207

---

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 768a476...029914a. Read the comment docs.

[erikras]

This pull request introduces 2 alerts when merging 029914a into 768a476 - view on LGTM.com

new alerts:

• 2 for Unused variable, import, function or class

---

Comment posted by LGTM.com

[Andarist]

@erikras new version of dtslint got published, this PR should be ready to merge now. Would be good to tweak LGTM setup to ignore rules such as no-unused-vars in typescript directory

[erikras]

Published in v4.1.0

[pokonski]

Hey @Andarist, can you tell why FieldRenderProps now requires the argument now?

[Andarist]

Truth to be told - I don't know. I don't recall making this change, but I've looked through git history and my local reflog and it seems I was indeed the one making it.

That being said the change makes sense to me FieldRenderProps["input"] is a set of closely related things, the generic should constraint the whole thing and not the specific handlers separately, because you could call them with different input type.

Could you paste in the example code which broke by this change? Could you describe why do you think this change should be reverted? We could now start writing tests to be sure such changes won't affect existing codebases.

However it's somewhat a known problem (and I dont think we can really improve on that as community) for libraries shipping type definitions - personally I dont think that typings change should require a major bump, it would be too restrictive. So the best approach is to lock your dependencies and update frequently.

[pokonski]

I just started using the library and noticed some TypeScript examples of how people write their components for fields and neither had the argument prior to the latest version of the react-final-form.

Is the argument really needed if it's not used for anything inside the interface?

[pokonski]

For example https://github.com/insxne/acid-forms/blob/c011b55a682ce459e9f027af70fe97e5bda9bdbb/src/LoginForm/TextField.tsx#L5

[Andarist]

Is the argument really needed if it's not used for anything inside the interface?

But it's used very much so - in event handlers declarations, i.e. here

Check out what this generic parameter gives u - https://github.com/final-form/react-final-form/pull/439/files#diff-c9d158e9961d9803798bff495584efabR1 . I could potentially add a default but it would be = HTMLElement, dunno if that would be useful.

[pokonski]

@Andarist ah indeed! It still feels a bit weird to have to specify it as an argument to those props because it basically can be either an input or a select, right?

For example Formik has it set to any:
https://github.com/jaredpalmer/formik/blob/02b8911f72554495afa18ddb87eaf1f4296d97ff/src/types.tsx#L140

as does ReduxForm:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/792f0a463bdc976ab1a22f405f183e2409df389d/types/redux-form/lib/Field.d.ts#L80

[Andarist]

@Andarist ah indeed! It still feels a bit weird to have to specify it as an argument to those props because it basically can be either an input or a select, right?

It could also be a textarea and probably (but not sure) a custom element (as in webcomponent).

In my opinion TS's goal is to type check our programs and it can only do that if we give it appropriate & strict hints. Using any anywhere is the opposite of safety and IMHO we should strive to avoid it at all costs - personally I try to use it only sometimes in very local places, casting it back to something conrete as soon as possible.

We could provide both versions under the same name using conditional types but it seems weird to me to allow non-strict versions because it probably will get used more frequently than strict one due to lack of exposure.

That being said we have to take developer ergonomics into account, I'm far from TS expert and I don't exactly know what are conventions used currently by the community, so would love having more people discussing this.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.