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

assignId: Assign non-object values to value property #940

Merged
merged 2 commits into from Jan 31, 2020

Conversation

ryanwalters
Copy link
Contributor

Possible solution to #924.

…ues, assign other values (e.g. strings, arrays) to value property
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 31, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d1173a9:

Sandbox Source
heuristic-morning-ks6fl Configuration

@@ -9,7 +10,7 @@ export const appendId = <
>(
value: FormArrayValues,
) => ({
...value,
...(isObject(value) ? { ...value } : { value }),
Copy link
Member

Choose a reason for hiding this comment

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

i guess this will still return

{
 id: 'xxx',
 value: 'xxx'
}

you cool with that?

Copy link
Member

Choose a reason for hiding this comment

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

i mean with fields object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! It keeps the shape consistent for non-object values, which is perfectly acceptable to me.

Copy link
Member

Choose a reason for hiding this comment

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

the improvement is you can do append('test') instead of append({ value: 'test' }), also hard code the data as value probably not the best thing as well.

Copy link
Member

Choose a reason for hiding this comment

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

i guess documentation probably can help on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is not with append, it's with setting defaultValues. Currently, string values get spread, which creates a crazy looking object like:

{
  "0": "a",
  "1": "b",
  "2": "c",
  "3": ".",
  "4": "c",
  "5": "o",
  "6": "m",
  "id": "5f0137bf-e201-425e-a7b0-f6cf76e0b3ca"
}

Copy link
Member

Choose a reason for hiding this comment

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

oh crap.

Copy link
Member

Choose a reason for hiding this comment

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

merge

src/logic/mapIds.ts Outdated Show resolved Hide resolved
Co-Authored-By: Bill <bluebill1049@hotmail.com>
@bluebill1049 bluebill1049 merged commit e9abd8f into react-hook-form:master Jan 31, 2020
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.

None yet

2 participants