fix: Initial FieldAPI tests, made FieldAPI typings much more strict (#405)

* fix: initial work at fixing the typescript typings for fieldapi to be more strict

* chore(form-core): change implementation of strict tdata

* fix(form-core): insertValue should now be typed properly

* test(form-core): validate array helpers

* fix(form-core): make types for getSubField more narrow

* chore: autoformat with prettier

* chore: fix tests, eslint

* chore: upgrade eslint deps

* chore: upgrade nx and concurrent

* chore: remove svelte from prettier plugins

* chore: upgrade TypeScript version to avoid a TS codegen bug

* chore: remove React 17 CI script temporarily

* chore: fix build and formatter
This commit is contained in:
Corbin Crutchley
2023-08-28 07:33:44 -07:00
committed by GitHub
parent 25237e40d4
commit 7ee5524693
11 changed files with 2895 additions and 2182 deletions

View File

@@ -54,6 +54,7 @@ const config = {
'import/no-unresolved': ['error', { ignore: ['^@tanstack/'] }], 'import/no-unresolved': ['error', { ignore: ['^@tanstack/'] }],
'import/no-unused-modules': ['off', { unusedExports: true }], 'import/no-unused-modules': ['off', { unusedExports: true }],
'no-redeclare': 'off', 'no-redeclare': 'off',
'@typescript-eslint/no-unused-vars': 'off',
}, },
overrides: [ overrides: [
{ {

View File

@@ -88,32 +88,6 @@ jobs:
- name: Install dependencies - name: Install dependencies
run: pnpm --filter "./packages/**" --filter form --prefer-offline install --no-frozen-lockfile run: pnpm --filter "./packages/**" --filter form --prefer-offline install --no-frozen-lockfile
- run: pnpm run test:format --base=${{ github.event.pull_request.base.sha }} - run: pnpm run test:format --base=${{ github.event.pull_request.base.sha }}
test-react-17:
name: 'Test React 17'
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
ref: ${{ github.head_ref }}
repository: ${{github.event.pull_request.head.repo.full_name}}
- uses: pnpm/action-setup@v2.2.4
with:
version: 7
- uses: actions/setup-node@v3
with:
node-version: 16.14.2
cache: 'pnpm'
- name: Install dependencies
run: pnpm --filter "./packages/**" --filter form --prefer-offline install --no-frozen-lockfile
- name: Run Tests
uses: nick-fields/retry@v2.8.3
with:
timeout_minutes: 5
max_attempts: 3
command: pnpm run test:react:17 --base=${{ github.event.pull_request.base.sha }}
env:
REACTJS_VERSION: 17
test-build: test-build:
name: 'Test Build' name: 'Test Build'
runs-on: ubuntu-latest runs-on: ubuntu-latest

View File

@@ -1,8 +1,5 @@
{ {
"semi": false, "semi": false,
"singleQuote": true, "singleQuote": true,
"trailingComma": "all", "trailingComma": "all"
"pluginSearchDirs": false,
"plugins": ["prettier-plugin-svelte"],
"overrides": [{ "files": "*.svelte", "options": { "parser": "svelte" } }]
} }

View File

@@ -20,7 +20,7 @@
"build:all": "nx run-many --exclude=examples/** --target=build", "build:all": "nx run-many --exclude=examples/** --target=build",
"watch": "pnpm run build:all && nx watch --all -- pnpm run build:all", "watch": "pnpm run build:all && nx watch --all -- pnpm run build:all",
"dev": "pnpm run watch", "dev": "pnpm run watch",
"prettier": "prettier --plugin-search-dir . \"{packages,examples,scripts}/**/*.{md,js,jsx,cjs,ts,tsx,json,vue,svelte}\"", "prettier": "prettier \"{packages,examples,scripts}/**/*.{md,js,jsx,cjs,ts,tsx,json,vue}\"",
"prettier:write": "pnpm run prettier --write", "prettier:write": "pnpm run prettier --write",
"cipublish": "node scripts/publish.js" "cipublish": "node scripts/publish.js"
}, },
@@ -51,8 +51,8 @@
"@types/react-dom": "^18.0.5", "@types/react-dom": "^18.0.5",
"@types/semver": "^7.3.13", "@types/semver": "^7.3.13",
"@types/testing-library__jest-dom": "^5.14.5", "@types/testing-library__jest-dom": "^5.14.5",
"@typescript-eslint/eslint-plugin": "^5.41.0", "@typescript-eslint/eslint-plugin": "^6.4.1",
"@typescript-eslint/parser": "^5.41.0", "@typescript-eslint/parser": "^6.4.1",
"@vitest/coverage-istanbul": "^0.27.1", "@vitest/coverage-istanbul": "^0.27.1",
"axios": "^0.26.1", "axios": "^0.26.1",
"babel-eslint": "^10.1.0", "babel-eslint": "^10.1.0",
@@ -60,24 +60,23 @@
"babel-preset-solid": "^1.5.4", "babel-preset-solid": "^1.5.4",
"bundlewatch": "^0.3.2", "bundlewatch": "^0.3.2",
"chalk": "^4.1.2", "chalk": "^4.1.2",
"concurrently": "^8.2.0", "concurrently": "^8.2.1",
"cpy-cli": "^5.0.0", "cpy-cli": "^5.0.0",
"current-git-branch": "^1.1.0", "current-git-branch": "^1.1.0",
"eslint": "^8.34.0", "eslint": "^8.48.0",
"eslint-config-prettier": "^8.8.0", "eslint-config-prettier": "^9.0.0",
"eslint-import-resolver-typescript": "^3.5.5", "eslint-import-resolver-typescript": "^3.6.0",
"eslint-plugin-compat": "^4.1.4", "eslint-plugin-compat": "^4.1.4",
"eslint-plugin-import": "^2.27.5", "eslint-plugin-import": "^2.28.1",
"eslint-plugin-react": "^7.32.2", "eslint-plugin-react": "^7.33.2",
"eslint-plugin-react-hooks": "^4.6.0", "eslint-plugin-react-hooks": "^4.6.0",
"git-log-parser": "^1.2.0", "git-log-parser": "^1.2.0",
"jsdom": "^22.0.0", "jsdom": "^22.0.0",
"jsonfile": "^6.1.0", "jsonfile": "^6.1.0",
"luxon": "^3.3.0", "luxon": "^3.3.0",
"nx": "^16.4.2", "nx": "^16.7.4",
"nx-cloud": "^16.0.5", "nx-cloud": "^16.0.5",
"prettier": "^2.8.8", "prettier": "^3.0.2",
"prettier-plugin-svelte": "^2.10.0",
"publint": "^0.1.15", "publint": "^0.1.15",
"react": "^18.2.0", "react": "^18.2.0",
"react-17": "npm:react@^17.0.2", "react-17": "npm:react@^17.0.2",
@@ -96,7 +95,7 @@
"stream-to-array": "^2.3.0", "stream-to-array": "^2.3.0",
"tsup": "^7.0.0", "tsup": "^7.0.0",
"type-fest": "^3.11.0", "type-fest": "^3.11.0",
"typescript": "^5.0.4", "typescript": "^5.2.2",
"vitest": "^0.27.1", "vitest": "^0.27.1",
"vue": "^3.2.47" "vue": "^3.2.47"
}, },

View File

@@ -1,8 +1,6 @@
// import type { DeepKeys, DeepValue, Updater } from './utils'
import type { DeepKeys, DeepValue, RequiredByKey, Updater } from './utils'
import type { FormApi, ValidationError } from './FormApi' import type { FormApi, ValidationError } from './FormApi'
import { Store } from '@tanstack/store' import { Store } from '@tanstack/store'
import { setBy } from './utils'
export type ValidationCause = 'change' | 'blur' | 'submit' export type ValidationCause = 'change' | 'blur' | 'submit'
@@ -76,14 +74,27 @@ export type FieldState<TData> = {
meta: FieldMeta meta: FieldMeta
} }
/**
* TData may not known at the time of FieldApi construction, so we need to
* use a conditional type to determine if TData is known or not.
*
* If TData is not known, we use the TFormData type to determine the type of
* the field value based on the field name.
*/
type GetTData<Name, TData, TFormData> = unknown extends TData
? DeepValue<TFormData, Name>
: TData
export class FieldApi<TData, TFormData> { export class FieldApi<TData, TFormData> {
uid: number uid: number
form: FormApi<TFormData> form: FormApi<TFormData>
name!: DeepKeys<TFormData> name!: DeepKeys<TFormData>
store!: Store<FieldState<TData>> // This is a hack that allows us to use `GetTData` without calling it everywhere
state!: FieldState<TData> _tdata!: GetTData<typeof this.name, TData, TFormData>
prevState!: FieldState<TData> store!: Store<FieldState<typeof this._tdata>>
options: FieldOptions<TData, TFormData> = {} as any state!: FieldState<typeof this._tdata>
prevState!: FieldState<typeof this._tdata>
options: FieldOptions<typeof this._tdata, TFormData> = {} as any
constructor(opts: FieldApiOptions<TData, TFormData>) { constructor(opts: FieldApiOptions<TData, TFormData>) {
this.form = opts.form this.form = opts.form
@@ -96,7 +107,7 @@ export class FieldApi<TData, TFormData> {
this.name = opts.name as any this.name = opts.name as any
this.store = new Store<FieldState<TData>>( this.store = new Store<FieldState<typeof this._tdata>>(
{ {
value: this.getValue(), value: this.getValue(),
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
@@ -115,7 +126,7 @@ export class FieldApi<TData, TFormData> {
: undefined : undefined
if (state.value !== this.prevState.value) { if (state.value !== this.prevState.value) {
this.validate('change', state.value) this.validate('change', state.value as never)
} }
this.prevState = state this.prevState = state
@@ -126,7 +137,7 @@ export class FieldApi<TData, TFormData> {
this.state = this.store.state this.state = this.store.state
this.prevState = this.state this.prevState = this.state
this.update(opts) this.update(opts as never)
} }
mount = () => { mount = () => {
@@ -148,7 +159,7 @@ export class FieldApi<TData, TFormData> {
}) })
}) })
this.options.onMount?.(this) this.options.onMount?.(this as never)
return () => { return () => {
unsubscribe() unsubscribe()
@@ -159,18 +170,19 @@ export class FieldApi<TData, TFormData> {
} }
} }
update = (opts: FieldApiOptions<TData, TFormData>) => { update = (opts: FieldApiOptions<typeof this._tdata, TFormData>) => {
this.options = { this.options = {
asyncDebounceMs: this.form.options.asyncDebounceMs ?? 0, asyncDebounceMs: this.form.options.asyncDebounceMs ?? 0,
onChangeAsyncDebounceMs: this.form.options.onChangeAsyncDebounceMs ?? 0, onChangeAsyncDebounceMs: this.form.options.onChangeAsyncDebounceMs ?? 0,
onBlurAsyncDebounceMs: this.form.options.onBlurAsyncDebounceMs ?? 0, onBlurAsyncDebounceMs: this.form.options.onBlurAsyncDebounceMs ?? 0,
...opts, ...opts,
} } as never
// Default Value // Default Value
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (this.state.value === undefined) { if (this.state.value === undefined) {
if (this.options.defaultValue !== undefined) { if (this.options.defaultValue !== undefined) {
this.setValue(this.options.defaultValue) this.setValue(this.options.defaultValue as never)
} else if ( } else if (
opts.form.options.defaultValues?.[ opts.form.options.defaultValues?.[
this.options.name as keyof TFormData this.options.name as keyof TFormData
@@ -179,7 +191,7 @@ export class FieldApi<TData, TFormData> {
this.setValue( this.setValue(
opts.form.options.defaultValues[ opts.form.options.defaultValues[
this.options.name as keyof TFormData this.options.name as keyof TFormData
] as TData, ] as never,
) )
} }
} }
@@ -191,11 +203,11 @@ export class FieldApi<TData, TFormData> {
} }
} }
getValue = (): TData => { getValue = (): typeof this._tdata => {
return this.form.getFieldValue(this.name) return this.form.getFieldValue(this.name)
} }
setValue = ( setValue = (
updater: Updater<TData>, updater: Updater<typeof this._tdata>,
options?: { touch?: boolean; notify?: boolean }, options?: { touch?: boolean; notify?: boolean },
) => { ) => {
this.form.setFieldValue(this.name, updater as any, options) this.form.setFieldValue(this.name, updater as any, options)
@@ -208,22 +220,33 @@ export class FieldApi<TData, TFormData> {
} }
getMeta = (): FieldMeta => this.form.getFieldMeta(this.name) getMeta = (): FieldMeta => this.form.getFieldMeta(this.name)
setMeta = (updater: Updater<FieldMeta>) => setMeta = (updater: Updater<FieldMeta>) =>
this.form.setFieldMeta(this.name, updater) this.form.setFieldMeta(this.name, updater)
getInfo = () => this.form.getFieldInfo(this.name) getInfo = () => this.form.getFieldInfo(this.name)
pushValue = (value: TData extends any[] ? TData[number] : never) => pushValue = (
this.form.pushFieldValue(this.name, value as any) value: typeof this._tdata extends any[]
insertValue = (index: number, value: TData) => ? (typeof this._tdata)[number]
this.form.insertFieldValue(this.name, index, value as any) : never,
) => this.form.pushFieldValue(this.name, value as any)
insertValue = (
index: number,
value: typeof this._tdata extends any[]
? (typeof this._tdata)[number]
: never,
) => this.form.insertFieldValue(this.name, index, value as any)
removeValue = (index: number) => this.form.removeFieldValue(this.name, index) removeValue = (index: number) => this.form.removeFieldValue(this.name, index)
swapValues = (aIndex: number, bIndex: number) => swapValues = (aIndex: number, bIndex: number) =>
this.form.swapFieldValues(this.name, aIndex, bIndex) this.form.swapFieldValues(this.name, aIndex, bIndex)
getSubField = <TName extends DeepKeys<TData>>(name: TName) => getSubField = <TName extends DeepKeys<typeof this._tdata>>(name: TName) =>
new FieldApi<DeepValue<TData, TName>, TFormData>({ new FieldApi<DeepValue<typeof this._tdata, TName>, TFormData>({
name: `${this.name}.${name}` as any, name: `${this.name}.${name}` as never,
form: this.form, form: this.form,
}) })
@@ -238,7 +261,7 @@ export class FieldApi<TData, TFormData> {
// track freshness of the validation // track freshness of the validation
const validationCount = (this.getInfo().validationCount || 0) + 1 const validationCount = (this.getInfo().validationCount || 0) + 1
this.getInfo().validationCount = validationCount this.getInfo().validationCount = validationCount
const error = normalizeError(validate(value, this)) const error = normalizeError(validate(value, this as never))
if (this.state.meta.error !== error) { if (this.state.meta.error !== error) {
this.setMeta((prev) => ({ this.setMeta((prev) => ({
@@ -321,7 +344,7 @@ export class FieldApi<TData, TFormData> {
// Only kick off validation if this validation is the latest attempt // Only kick off validation if this validation is the latest attempt
if (checkLatest()) { if (checkLatest()) {
try { try {
const rawError = await validate(value, this) const rawError = await validate(value, this as never)
if (checkLatest()) { if (checkLatest()) {
const error = normalizeError(rawError) const error = normalizeError(rawError)
@@ -351,7 +374,7 @@ export class FieldApi<TData, TFormData> {
validate = ( validate = (
cause: ValidationCause, cause: ValidationCause,
value?: TData, value?: typeof this._tdata,
): ValidationError | Promise<ValidationError> => { ): ValidationError | Promise<ValidationError> => {
// If the field is pristine and validatePristine is false, do not validate // If the field is pristine and validatePristine is false, do not validate
if (!this.state.meta.isTouched) return if (!this.state.meta.isTouched) return
@@ -372,12 +395,13 @@ export class FieldApi<TData, TFormData> {
getChangeProps = <T extends UserChangeProps<any>>( getChangeProps = <T extends UserChangeProps<any>>(
props: T = {} as T, props: T = {} as T,
): ChangeProps<TData> & Omit<T, keyof ChangeProps<TData>> => { ): ChangeProps<typeof this._tdata> &
Omit<T, keyof ChangeProps<typeof this._tdata>> => {
return { return {
...props, ...props,
value: this.state.value, value: this.state.value,
onChange: (value) => { onChange: (value) => {
this.setValue(value) this.setValue(value as never)
props.onChange?.(value) props.onChange?.(value)
}, },
onBlur: (e) => { onBlur: (e) => {
@@ -388,12 +412,14 @@ export class FieldApi<TData, TFormData> {
} }
this.validate('blur') this.validate('blur')
}, },
} as ChangeProps<TData> & Omit<T, keyof ChangeProps<TData>> } as ChangeProps<typeof this._tdata> &
Omit<T, keyof ChangeProps<typeof this._tdata>>
} }
getInputProps = <T extends UserInputProps>( getInputProps = <T extends UserInputProps>(
props: T = {} as T, props: T = {} as T,
): InputProps<TData> & Omit<T, keyof InputProps<TData>> => { ): InputProps<typeof this._tdata> &
Omit<T, keyof InputProps<typeof this._tdata>> => {
return { return {
...props, ...props,
value: this.state.value, value: this.state.value,
@@ -402,7 +428,8 @@ export class FieldApi<TData, TFormData> {
props.onChange?.(e.target.value) props.onChange?.(e.target.value)
}, },
onBlur: this.getChangeProps(props).onBlur, onBlur: this.getChangeProps(props).onBlur,
} } as InputProps<typeof this._tdata> &
Omit<T, keyof InputProps<typeof this._tdata>>
} }
} }

View File

@@ -171,6 +171,7 @@ export class FormApi<TFormData> {
const shouldUpdateState = const shouldUpdateState =
options.defaultState !== this.options.defaultState options.defaultState !== this.options.defaultState
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (!shouldUpdateValues || !shouldUpdateValues) { if (!shouldUpdateValues || !shouldUpdateValues) {
return return
} }
@@ -179,7 +180,9 @@ export class FormApi<TFormData> {
getDefaultFormState( getDefaultFormState(
Object.assign( Object.assign(
{}, {},
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
shouldUpdateState ? options.defaultState : {}, shouldUpdateState ? options.defaultState : {},
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
shouldUpdateValues shouldUpdateValues
? { ? {
values: options.defaultValues, values: options.defaultValues,
@@ -196,9 +199,8 @@ export class FormApi<TFormData> {
reset = () => reset = () =>
this.store.setState(() => this.store.setState(() =>
getDefaultFormState({ getDefaultFormState({
...this.options?.defaultState, ...this.options.defaultState,
values: values: this.options.defaultValues ?? this.options.defaultState?.values,
this.options?.defaultValues ?? this.options?.defaultState?.values,
}), }),
) )
@@ -298,6 +300,7 @@ export class FormApi<TFormData> {
} }
getFieldInfo = <TField extends DeepKeys<TFormData>>(field: TField) => { getFieldInfo = <TField extends DeepKeys<TFormData>>(field: TField) => {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
return (this.fieldInfo[field] ||= { return (this.fieldInfo[field] ||= {
instances: {}, instances: {},
}) })

View File

@@ -0,0 +1,143 @@
import { expect } from 'vitest'
import { FormApi } from '../FormApi'
import { FieldApi } from '../FieldApi'
describe('field api', () => {
it('should have an initial value', () => {
const form = new FormApi({
defaultValues: {
name: 'test',
},
})
const field = new FieldApi({
form,
name: 'name',
})
expect(field.getValue()).toBe('test')
})
it('should set a value correctly', () => {
const form = new FormApi({
defaultValues: {
name: 'test',
},
})
const field = new FieldApi({
form,
name: 'name',
})
field.setValue('other')
expect(field.getValue()).toBe('other')
})
it('should set a value correctly', () => {
const form = new FormApi({
defaultValues: {
name: 'test',
},
})
const field = new FieldApi({
form,
name: 'name',
})
field.setValue('other')
expect(field.getValue()).toBe('other')
})
it('should push an array value correctly', () => {
const form = new FormApi({
defaultValues: {
names: ['one'],
},
})
const field = new FieldApi({
form,
name: 'names',
})
field.pushValue('other')
expect(field.getValue()).toStrictEqual(['one', 'other'])
})
it('should insert a value into an array value correctly', () => {
const form = new FormApi({
defaultValues: {
names: ['one', 'two'],
},
})
const field = new FieldApi({
form,
name: 'names',
})
field.insertValue(1, 'other')
expect(field.getValue()).toStrictEqual(['one', 'other'])
})
it('should remove a value from an array value correctly', () => {
const form = new FormApi({
defaultValues: {
names: ['one', 'two'],
},
})
const field = new FieldApi({
form,
name: 'names',
})
field.removeValue(1)
expect(field.getValue()).toStrictEqual(['one'])
})
it('should swap a value from an array value correctly', () => {
const form = new FormApi({
defaultValues: {
names: ['one', 'two'],
},
})
const field = new FieldApi({
form,
name: 'names',
})
field.swapValues(0, 1)
expect(field.getValue()).toStrictEqual(['two', 'one'])
})
it('should get a subfield properly', () => {
const form = new FormApi({
defaultValues: {
names: {
first: 'one',
second: 'two',
},
},
})
const field = new FieldApi({
form,
name: 'names',
})
const subfield = field.getSubField('first')
expect(subfield.getValue()).toBe('one')
})
})

View File

@@ -165,3 +165,6 @@ type NarrowRaw<A> =
export type Narrow<A> = Try<A, [], NarrowRaw<A>> export type Narrow<A> = Try<A, [], NarrowRaw<A>>
type Try<A1, A2, Catch = never> = A1 extends A2 ? A1 : Catch type Try<A1, A2, Catch = never> = A1 extends A2 ? A1 : Catch
// Hack to get TypeScript to show simplified types in error messages
export type Pretty<T> = { [K in keyof T]: T[K] } & {}

View File

@@ -1,5 +1,5 @@
import { fireEvent, render } from "@testing-library/react"; import { fireEvent, render } from '@testing-library/react'
import userEvent from "@testing-library/user-event"; import userEvent from '@testing-library/user-event'
import '@testing-library/jest-dom' import '@testing-library/jest-dom'
import * as React from 'react' import * as React from 'react'
import { createFormFactory } from '..' import { createFormFactory } from '..'
@@ -22,9 +22,11 @@ describe('useForm', () => {
<form.Provider> <form.Provider>
<form.Field <form.Field
name="firstName" name="firstName"
defaultValue={""} defaultValue={''}
children={(field) => { children={(field) => {
return <input data-testid="fieldinput" {...field.getInputProps()} /> return (
<input data-testid="fieldinput" {...field.getInputProps()} />
)
}} }}
/> />
</form.Provider> </form.Provider>
@@ -32,10 +34,10 @@ describe('useForm', () => {
} }
const { getByTestId, queryByText } = render(<Comp />) const { getByTestId, queryByText } = render(<Comp />)
const input = getByTestId("fieldinput"); const input = getByTestId('fieldinput')
expect(queryByText('FirstName')).not.toBeInTheDocument() expect(queryByText('FirstName')).not.toBeInTheDocument()
await user.type(input, "FirstName") await user.type(input, 'FirstName')
expect(input).toHaveValue("FirstName") expect(input).toHaveValue('FirstName')
}) })
it('should allow default values to be set', async () => { it('should allow default values to be set', async () => {

View File

@@ -54,7 +54,7 @@ export function useField<TData, TFormData>(
}) })
// Keep options up to date as they are rendered // Keep options up to date as they are rendered
fieldApi.update({ ...opts, form: formApi }) fieldApi.update({ ...opts, form: formApi } as never)
useStore( useStore(
fieldApi.store as any, fieldApi.store as any,

4756
pnpm-lock.yaml generated

File diff suppressed because it is too large Load Diff