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

Add nested objects and array support #4

Merged
merged 18 commits into from
Dec 13, 2021

Conversation

lucascurti
Copy link
Contributor

@airjp73 I still need to create some tests and work on the documentation updates but the nested objects and arrays should be working. It would be great if you can play a bit with it and let me know what you think. I didn't implemented the option to turn off nesting completely for a field using brackets. I think that one can be added in a later PR.

To run the sample-app just run npm run sample-app in the root folder.

I'm keeping this one as draft until documentation and tests are added.

Copy link
Owner

@airjp73 airjp73 left a comment

Choose a reason for hiding this comment

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

Thanks for the work! I booted up the sample app and everything seems to be working correctly. I had a few comments but largely looks good.

sample-app/app/components/SubjectForm.tsx Show resolved Hide resolved
sample-app/.env Show resolved Hide resolved
README.md Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/flatten.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/validation/validation.test.ts Show resolved Hide resolved
@airjp73 airjp73 self-requested a review December 12, 2021 22:19
@airjp73
Copy link
Owner

airjp73 commented Dec 12, 2021

Whoops, didn't mean to request a review from myself 😆

@lucascurti
Copy link
Contributor Author

@airjp73. Thanks for those comments! I implemented them and marked them as resolved. Check my replies and feel free to let me know if you have any other feedback. I'll work in the documentation and let you know as soon as it's ready.

@airjp73
Copy link
Owner

airjp73 commented Dec 12, 2021

It's looking good to me. 👍

lucascurti added 2 commits December 13, 2021 00:36
…e & downgrade to EsLint 7

I upgraded EsLint to v8 but is having some problems with ES Module. We can investigate it later
@lucascurti lucascurti marked this pull request as ready for review December 13, 2021 00:32
@lucascurti
Copy link
Contributor Author

lucascurti commented Dec 13, 2021

@airjp73 This one is now ready for review.

  • I've updated the README:
    • added info on nested objects and arrays,
    • uploaded a new Demo video from the sample-app
    • added instructions on how to run the sample-app
    • added information on createValidator
    • added FAQs section for the noValidate option.
  • I upgraded to EsLint 8 but it's having some issues when linting so I downgraded to v7 (the one you were originally using) and everything is good now. You might want to run those actions again and see if they pass now.

This PR is now longer in Draft so feel free to merge it when you are ready. 🥳

@airjp73
Copy link
Owner

airjp73 commented Dec 13, 2021

Looks good, thanks!

@airjp73 airjp73 merged commit 30b9a70 into airjp73:main Dec 13, 2021
@airjp73
Copy link
Owner

airjp73 commented Dec 13, 2021

@lucascurti, I just published the changes under 1.1.1-beta.0 in case you wanted to use it right away. I'm gonna work on the other issues I made before releasing so I can bump the major version.

@lucascurti
Copy link
Contributor Author

Cool! I'll update to the beta tomorrow.
BTW, in case you find it useful, I recommend https://intuit.github.io/auto/ for publishing to npm, creating release notes, beta/next versioning and a bunch of other things. It's pretty easy to setup.

@airjp73
Copy link
Owner

airjp73 commented Dec 13, 2021

I'll check it out 👍

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.

2 participants