refactor: tighten path validation when parsing formdata #983
Conversation
🦋 Changeset detectedLatest commit: 6eb1d04 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
More templates
@conform-to/dom
@conform-to/react
@conform-to/valibot
@conform-to/validitystate
@conform-to/yup
@conform-to/zod
commit: |
| const output2 = parseWithValibot(formData2, { schema: schema2 }); | ||
| expect(output2).toMatchObject({ | ||
| status: 'success', | ||
| value: { nest: [{ name: 'test name' }] }, | ||
| }); | ||
|
|
||
| const errorFormData = createFormData('nest[].name', ''); | ||
| const errorFormData = createFormData('nest[0].name', ''); |
There was a problem hiding this comment.
@chimame FYI: you may need to update the tests in your repo. We originally added empty-bracket support to make it easier to resolve validation attributes from the derived constraints, but empty brackets aren't officially supported when parsing formdata as it is ambiguous which item you are referring to in a nested object.
With this refactor, we have added limited empty-bracket support: if we see empty bracket at the end of a path (e.g. items[]), we will initialize an array and append the value. But empty brackets anywhere else in the path remain unsupported.
There was a problem hiding this comment.
Thank you for letting me know. 👍
I'll keep that in mind when I create tests in the future. I'll also try to change any existing tests that seem to need changing.
| getPathSegments as getPaths, | ||
| formatPathSegments as formatPaths, |
There was a problem hiding this comment.
I have renamed several helpers internally. But still exported in the old names with the exact same signature. So it shouldn't break anything.
| createGlobalFormsObserver as unstable_createGlobalFormsObserver, | ||
| focus as unstable_focus, | ||
| change as unstable_change, | ||
| blur as unstable_blur, |
There was a problem hiding this comment.
I moved all these exports to the future export so it's easier to manage as I put up more helpers that are only used by those future APIs.
| test('getPathSegments', () => { | ||
| expect(getPathSegments(undefined)).toEqual([]); | ||
| expect(getPathSegments('')).toEqual([]); | ||
| expect(getPathSegments('title')).toEqual(['title']); | ||
| expect(getPathSegments('123')).toEqual(['123']); | ||
| expect(getPathSegments('amount.currency')).toEqual(['amount', 'currency']); | ||
| expect(getPathSegments('[0]')).toEqual([0]); | ||
| expect(getPathSegments('tasks[0]')).toEqual(['tasks', 0]); | ||
| expect(getPathSegments('tasks[1].completed')).toEqual([ | ||
| 'tasks', | ||
| 1, | ||
| 'completed', | ||
| ]); | ||
| expect(getPathSegments('multiple[0][1][2]')).toEqual(['multiple', 0, 1, 2]); | ||
| expect(getPathSegments('books[0].chapters[1]')).toEqual([ | ||
| 'books', | ||
| 0, | ||
| 'chapters', | ||
| 1, | ||
| ]); | ||
| expect(getPathSegments('[].array[].prop')).toEqual(['', 'array', '', 'prop']); | ||
| expect(() => getPathSegments('a.b..c...d')).toThrow( | ||
| 'Invalid path syntax at position 3 in "a.b..c...d"', | ||
| ); | ||
| expect(() => getPathSegments('.')).toThrow( | ||
| 'Invalid path syntax at position 0 in "."', | ||
| ); | ||
| expect(() => getPathSegments('a[b]')).toThrow( | ||
| 'Invalid path syntax at position 1 in "a[b]"', | ||
| ); | ||
| expect(() => getPathSegments('[a.b]')).toThrow( | ||
| 'Invalid path syntax at position 0 in "[a.b]"', | ||
| ); | ||
| expect(() => getPathSegments('a[b].b')).toThrow( | ||
| 'Invalid path syntax at position 1 in "a[b].b"', | ||
| ); | ||
| expect(() => getPathSegments('a[[]]')).toThrow( | ||
| 'Invalid path syntax at position 1 in "a[[]]"', | ||
| ); | ||
| }); |
There was a problem hiding this comment.
This should make it clear what kind of paths we accept.
| expect(() => | ||
| setValueAtPath(target, 'foo.bar[].baz', 'test'), | ||
| ).toThrowError(); | ||
| expect(() => | ||
| setValueAtPath(target, 'foo.bar[].baz', 'test', { silent: true }), | ||
| ).not.toThrowError(); |
There was a problem hiding this comment.
Re: empty bracket support
edmundhung
force-pushed
the
edmundhung/refactor-dom-utils
branch
from
1e482a7 to
d2c9519
Compare
Deploying conform with
|
| Latest commit: |
6eb1d04
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5d2d7970.conform.pages.dev |
| Branch Preview URL: | https://edmundhung-refactor-dom-util.conform.pages.dev |
edmundhung
force-pushed
the
edmundhung/refactor-dom-utils
branch
from
d2c9519 to
1bf3788
Compare
To prepare for upcoming changes, we have overhauled the way paths are parsed and formatted:
This shouldn't have any impact to existing users. Users who rely on inferred names from field metadata, or who supply explicit names using dot-and-bracket notation, will see no change in behavior.