Skip to content

fix: submit form after formik values are saved #15430

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 1 commit into from
Apr 15, 2025
Merged

fix: submit form after formik values are saved #15430

merged 1 commit into from
Apr 15, 2025

Conversation

oxaudo
Copy link
Member

@oxaudo oxaudo commented Apr 10, 2025

Attempting to fix: EMI-2376

After some digging and pointers from @erikdstock seems like the issue is that submit form is called before formik.setValues finished executing. And seems like setValues doesn't return a promise, awaiting it doesn't ensure the values are updated. - so trying to use useEffect with a dependency on formikContext.values instead when address is saved.

Seems to fix this concrete error but not sure it does not break something else :( Did some local testing and seems fine though....

@oxaudo oxaudo requested a review from erikdstock April 10, 2025 20:51
@oxaudo oxaudo self-assigned this Apr 10, 2025
Copy link

relativeci bot commented Apr 10, 2025

#2354 Bundle Size — 8.74MiB (-0.31%).

8ab6114(current) vs 980b7ae main#2351(baseline)

Warning

Bundle contains 18 duplicate packages – View duplicate packages

Bundle metrics  Change 8 changes Regression 1 regression Improvement 2 improvements
                 Current
#2354
     Baseline
#2351
Improvement  Initial JS 3.55MiB(-0.7%) 3.58MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 48.16% 40.94%
Change  Chunks 101(-0.98%) 102
Change  Assets 104(-0.95%) 105
Change  Modules 5743(+0.05%) 5740
Regression  Duplicate Modules 524(+1.35%) 517
Change  Duplicate Code 4.08%(+1.49%) 4.02%
Improvement  Packages 269(-0.74%) 271
No change  Duplicate Packages 17 17
Bundle size by type  Change 2 changes Improvement 2 improvements
                 Current
#2354
     Baseline
#2351
Improvement  JS 8.6MiB (-0.3%) 8.63MiB
Improvement  Other 141.62KiB (-0.83%) 142.8KiB

Bundle analysis reportBranch oxaudo/addressProject dashboard


Generated by RelativeCIDocumentationReport issue

@erikdstock
Copy link
Contributor

And seems like setValues doesn't return a promise, awaiting it doesn't ensure the values are updated. - so trying to use useEffect with a dependency on formikContext.values instead when address is saved.

setValues() does return a promise but we could confirm it with our version of formik. I don't see what difference this code would make in that issue except that adding some extra useState + useEffect will add some more async magic that might be waiting long enough. TLDR I think that if this works it is by luck and we are hacking around some deeper problem. Maybe with the upcoming work we have that is enough for a short-term fix. Interested in others' thoughts.

I do see that the setValues call will also call the validation logic - i wonder if something could be going wrong in a way we don't see with the nested saved addresses form values going into the fulfillment details context.

@oxaudo
Copy link
Member Author

oxaudo commented Apr 14, 2025

I'm going to merge this one and see how it performs. Any objections?

@oxaudo oxaudo merged commit 050c6e8 into main Apr 15, 2025
11 checks passed
@oxaudo oxaudo deleted the oxaudo/address branch April 15, 2025 17:28
@artsy-peril artsy-peril bot mentioned this pull request Apr 15, 2025
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