Skip to content

[typescript] add tests and implementation for FieldArrayProps #18

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

Merged
merged 10 commits into from
Jan 23, 2018

Conversation

rosskevin
Copy link
Contributor

@rosskevin rosskevin commented Jan 22, 2018

@rosskevin
Copy link
Contributor Author

Once versions are bumped for final-form/final-form-arrays#8 and final-form/react-final-form#129, this one will be good to go.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@erikras
Copy link
Member

erikras commented Jan 23, 2018

@rosskevin
Copy link
Contributor Author

I'll check it out

@rosskevin
Copy link
Contributor Author

This should have been fixed by my change to react-final-form. FormRenderProps.mutatators should be defined, not undefined. https://github.com/final-form/react-final-form/pull/129/files#diff-b739f95ace6bbf2f0fd5e8cf1dab97d8R47

Checking now to see what's the deal with the release.

@rosskevin
Copy link
Contributor Author

rosskevin commented Jan 23, 2018

@erikras react-final-form needs a release, then needs bumped here.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@rosskevin
Copy link
Contributor Author

@erikras - I'm not sure why there are unrelated test failures. If you can take a look, I would appreciate it.

On an unrelated note, I found more ts inaccuracies (here) - I'll work on that now, let's leave this open.

@rosskevin
Copy link
Contributor Author

@erikras FieldState (e.g. active | blur) is being picked up in ...meta, so either the runtime is wrong or the types are wrong. where should FieldState be in the FieldArrayRenderProps?

@rosskevin
Copy link
Contributor Author

rosskevin commented Jan 23, 2018

Snapshot of FieldState mixed into render props (all but value under meta):

fieldstate

@rosskevin
Copy link
Contributor Author

@erikras I think all FieldState values should be moved under fields. The types imply it already and I think that makes sense. Should I make that change?

@erikras
Copy link
Member

erikras commented Jan 23, 2018

Should I make that change?

mnee7

@rosskevin
Copy link
Contributor Author

rosskevin commented Jan 23, 2018

@erikras any ideas about the test failure? https://travis-ci.org/final-form/react-final-form-arrays/builds/332369245

Seems like the upstream dependencies changed and are causing a failure on the field-level validation test (that's speculation btw)

rosskevin and others added 5 commits January 23, 2018 12:18

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@codecov
Copy link

codecov bot commented Jan 23, 2018

Codecov Report

Merging #18 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #18   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines          64     66    +2     
  Branches       16     15    -1     
=====================================
+ Hits           64     66    +2
Impacted Files Coverage Δ
src/FieldArray.js 100% <100%> (ø) ⬆️

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 4e128e2...c3c82e4. Read the comment docs.

@erikras
Copy link
Member

erikras commented Jan 23, 2018

Fixed it.

@rosskevin
Copy link
Contributor Author

Great, thank you. This one is good to merge/release

@erikras erikras merged commit c44ad1d into final-form:master Jan 23, 2018
@erikras
Copy link
Member

erikras commented Jan 23, 2018

Published in v1.0.3.

@rosskevin rosskevin deleted the ts-fixes branch January 23, 2018 18:52
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

2 participants