-
-
Notifications
You must be signed in to change notification settings - Fork 355
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
TanStack Store Internals Refactor #1038
Conversation
View your CI Pipeline Execution ↗ for commit 1c360a6.
☁️ Nx Cloud last updated this comment at |
# Conflicts: # docs/framework/angular/reference/classes/tanstackfield.md # docs/framework/react/reference/functions/useform.md # docs/framework/react/reference/interfaces/reactformapi.md # docs/framework/react/reference/type-aliases/fieldcomponent.md # docs/framework/react/reference/type-aliases/reactformextendedapi.md # docs/framework/vue/reference/functions/usefield.md # docs/framework/vue/reference/functions/useform.md # docs/framework/vue/reference/interfaces/vuefieldapi.md # docs/framework/vue/reference/interfaces/vueformapi.md # docs/framework/vue/reference/type-aliases/fieldcomponent.md # docs/framework/vue/reference/type-aliases/usefield.md # docs/framework/vue/reference/variables/field.md # docs/reference/classes/fieldapi.md # docs/reference/classes/formapi.md # docs/reference/type-aliases/fieldmeta.md # docs/reference/type-aliases/fieldstate.md # packages/form-core/src/FieldApi.ts # packages/form-core/src/FormApi.ts # pnpm-lock.yaml
OK, I figured out what the problem was:
function batch(fn: () => void) {
return fn()
} It works as-intended and all tests pass. This isn't ideal tho, so I'll try to patch it in |
# Conflicts: # docs/reference/classes/fieldapi.md # docs/reference/classes/formapi.md # docs/reference/type-aliases/fieldmeta.md # docs/reference/type-aliases/fieldstate.md # docs/reference/type-aliases/formstate.md # examples/react/next-server-actions/package.json # examples/react/remix/package.json # examples/react/tanstack-start/package.json # packages/angular-form/package.json # packages/form-core/package.json # packages/form-core/src/FieldApi.ts # packages/form-core/src/FormApi.ts # packages/react-form/package.json # packages/solid-form/package.json # packages/vue-form/package.json # pnpm-lock.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see there are also some failing tests on Solid which seem to be related to the initial form state, I couldn't see if it's something on the solid-form adapter or even from solid-store
!prevBaseStore || | ||
currBaseStore.errorMap !== prevBaseStore.errorMap | ||
) { | ||
errors = Object.values(currBaseStore.errorMap).reduce( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we check earlier if onMount
errors have to be removed? So that we avoid pushing them now and then filtering them out on lines 515-521
Something like moving up this block
let errorMap = currBaseStore.errorMap
if (shouldInvalidateOnMount) {
errorMap = Object.assign(errorMap, { onMount: undefined })
}
and then using the local errorMap
on Object.values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make a follow-up PR on this, seeing that it works today and I'm struggling to think of an easy way to move this around ATM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1038 +/- ##
==========================================
- Coverage 86.61% 86.60% -0.02%
==========================================
Files 29 29
Lines 1158 1172 +14
Branches 301 290 -11
==========================================
+ Hits 1003 1015 +12
- Misses 142 144 +2
Partials 13 13 ☔ View full report in Codecov by Sentry. |
TODOs
new Derived
side effects to dedicatedmount
methodtransformArray
errors
doesn't change unlesserrorMap
has changed