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

feat: Formz validator now allows async validation #93

Closed

Conversation

gabrielchaves7
Copy link

Description

I've added code to support async form validations.
Initially I just want to bring to discussion on how to do it. Not exactly want the initial PR to be merged.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@jmsandiegoo
Copy link

Any updates regarding this?

@gabrielchaves7
Copy link
Author

Any updates regarding this?

I don't know if this is the right approach to do it, I opened the PR to start the discussion with the guys from VGV, but they didn't leave any comment yet

@renancaraujo
Copy link
Contributor

renancaraujo commented Jul 18, 2023

Adding reference to the issue here: #15

@gabrielchaves7
Copy link
Author

I would love to implement this one, I'm just waiting the review guys

@renancaraujo renancaraujo self-assigned this Sep 19, 2023
@renancaraujo renancaraujo removed their assignment Oct 4, 2023
@tomarra
Copy link
Contributor

tomarra commented Oct 26, 2023

@gabrielchaves7 it looks like there is a merge conflict. Can you please resolve?

@alestiago alestiago added the waiting for response Waiting for follow up label Oct 27, 2023
@gabrielchaves7 gabrielchaves7 requested a review from a team as a code owner November 13, 2023 00:00
@gabrielchaves7
Copy link
Author

gabrielchaves7 commented Nov 13, 2023

@tomarra @alestiago Just fixed the conflicts, sorry for the delay >_<

@alestiago alestiago removed the waiting for response Waiting for follow up label Nov 13, 2023
@tomarra tomarra changed the title Formz validator now allow async validation feat: Formz validator now allows async validation Nov 14, 2023
@gabrielchaves7
Copy link
Author

gabrielchaves7 commented Nov 16, 2023

All checks passing, can someone take a look?

@alestiago
Copy link
Contributor

Thanks for the contribution @gabrielchaves7 💙. I would like to give it a look. However, you'll have to bear with me a bit. Sorry for the delay.

cc: @wolfenrain , @renancaraujo

@gabrielchaves7
Copy link
Author

don't worry, feel free to point anything

lib/formz.dart Outdated Show resolved Hide resolved
example/lib/main.dart Outdated Show resolved Hide resolved
test/formz_test.dart Outdated Show resolved Hide resolved
@alestiago alestiago self-assigned this Nov 27, 2023
@alestiago alestiago added the waiting for response Waiting for follow up label Nov 27, 2023
@gabrielchaves7
Copy link
Author

@alestiago Can you review again?

@gabrielchaves7
Copy link
Author

Thanks for the review @alestiago really appreciate it :)

@gabrielchaves7
Copy link
Author

What are the next steps here?

@wolfenrain
Copy link
Member

What are the next steps here?

Hi! Me and @alestiago just had a look at the PR and I think we need re-evaluate the solution of the PR's goal. formz in its current state can be used in a broad set of scenarios and this PR as it is will limit them.

I do think that async validation is a necessity but we need to think about how we will introduce this with as little as possible breaking changes both in the API and the scenarios.

@alestiago alestiago removed the waiting for response Waiting for follow up label Dec 1, 2023
@gabrielchaves7
Copy link
Author

  • What do you think it should be re evaluate?
  • Which breaking change (s) do you think we should try to avoid?

@wolfenrain
Copy link
Member

  • What do you think it should be re evaluate?
  • Which breaking change (s) do you think we should try to avoid?

If anything we would prefer to prevent any breaking changes, which might mean we just need to provide a new API on top of the existing that is async ready, assuming we can make that work use-cases such as using the FormzInputs directly in the build tree.

Formz is simple enough that people can use it in versatile ways and we would love to ensure that we dont break that simplicity which requires us to evaluate our options, so all I can say right now is that we do want to provide async validation.

If you (or anyone else) want/need it right now you can always make an API on top of Formz that is asynchronous. That is also the beauty of Formz.

@tomarra
Copy link
Contributor

tomarra commented Dec 11, 2023

Hi @gabrielchaves7 looking to get your feedback here as we are attempting to cleanup open PR's across our packages. Given the last comment from @wolfenrain is this something you plan on still pursuing?

@gabrielchaves7
Copy link
Author

gabrielchaves7 commented Dec 17, 2023

Right now I don't intend to move on with this PR. If I come up with any implementation that can address @wolfenrain then I will open a new PR. I really appreciated the review you did but I think we did a bad job on scoping. I waste my time and your time just bc we did not discuss the implementation enough before implementing it and I apologize for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants