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

#220 Upgrade mantine packages #223

Merged
merged 17 commits into from
Aug 13, 2024
Merged

Conversation

nevendyulgerov
Copy link
Contributor

@nevendyulgerov nevendyulgerov commented Jul 31, 2024

I upgraded all mantine packages to their latest versions in all apps/packages that use them and tested for regressions.

In regards to the @mantine/forms package, I noticed a runtime error on the /specifications/new page that was causing the page to break. The issue was coming from the form.setFieldValue function being passed as a dependency to other hooks. Removing it from the dependency array solved that issue. More on this - here.

Another thing I noticed was issues with some forms using setValues function. This function was causing the validation to be reset for the form resulting in error messages disappearing unexpectedly. Some unit tests were failing because of this. Switching to setFieldValue solved that issue.

@nevendyulgerov nevendyulgerov self-assigned this Jul 31, 2024
Copy link

vercel bot commented Jul 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rollups-explorer-base-mainnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2024 11:28am
rollups-explorer-base-sepolia 🛑 Canceled (Inspect) Aug 9, 2024 11:28am
rollups-explorer-mainnet 🛑 Canceled (Inspect) Aug 9, 2024 11:28am
rollups-explorer-optimism-mainnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2024 11:28am
rollups-explorer-optimism-sepolia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2024 11:28am
rollups-explorer-sepolia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2024 11:28am
rollups-explorer-workshop ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2024 11:28am

@nevendyulgerov nevendyulgerov linked an issue Jul 31, 2024 that may be closed by this pull request
4 tasks
@nevendyulgerov nevendyulgerov added the Type: Chore Improvements to the code base and DX. It has no impact on external users e.g. Update deps label Jul 31, 2024
@nevendyulgerov nevendyulgerov marked this pull request as ready for review August 1, 2024 10:30
@brunomenezes
Copy link
Collaborator

@nevendyulgerov , the PR need a rebase. It may need to address other parts for the Forms changes since new features were added using the Mantine form.

@nevendyulgerov nevendyulgerov force-pushed the feature/220-update-mantine-packages branch from 4a0be0b to ff805b1 Compare August 2, 2024 14:37
@nevendyulgerov nevendyulgerov force-pushed the feature/220-update-mantine-packages branch from 55755dc to cb66073 Compare August 9, 2024 08:25

if (transformedValues.formMode === "EDITION" && !initialValRef.current)
const setFieldValue = useCallback((name: string, value: unknown) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to cache this function, otherwise it was breaking the specifications page due to an infinite state update loop.

@@ -307,7 +307,8 @@ const DepositFormSingle: FC<Props> = (props) => {
"erc1155Address",
formattedValue,
);
form.setValues({ amount: "", tokenId: "" });
form.setFieldValue("amount", "");
Copy link
Contributor Author

@nevendyulgerov nevendyulgerov Aug 9, 2024

Choose a reason for hiding this comment

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

Using setValues here was causing the errors to disappear like so:

Screen.Recording.2024-08-09.at.11.37.11.mov

Switching to setFieldValue mitigates the issue:

Screen.Recording.2024-08-09.at.13.57.01.mov

Copy link
Collaborator

@brunomenezes brunomenezes left a comment

Choose a reason for hiding this comment

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

Nice! It is sad that a few regressions were introduced. I believe on version 7.8.x is where most were added. Before, setValues() would have the same result as Object.assign(target, source). Nonetheless, I think the changes are better by addressing only the bits causing troubles rather than changing the validation behaviour on every form.

@nevendyulgerov nevendyulgerov merged commit f218d67 into main Aug 13, 2024
20 checks passed
@nevendyulgerov nevendyulgerov deleted the feature/220-update-mantine-packages branch August 13, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Chore Improvements to the code base and DX. It has no impact on external users e.g. Update deps
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update mantine packages
2 participants