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

My get/set PR seems to have hurt performance #442

Open
jedwards1211 opened this issue Dec 10, 2021 · 0 comments
Open

My get/set PR seems to have hurt performance #442

jedwards1211 opened this issue Dec 10, 2021 · 0 comments

Comments

@jedwards1211
Copy link
Contributor

jedwards1211 commented Dec 10, 2021

Are you submitting a bug report or a feature request?

Bug Report

What is the current behavior?

It appears that get and setIn are now quite slow, probably thanks to the full JS path syntax support in my merged PR. (See screenshot) I didn't think about that risk...

Given that I copied the get/set code from lodash, I assume it's pretty optimized already and supporting full JS pathstrings with high performance may be intractable. The lodash code even caches parsed paths, but this doesn't help the first time a field is seen; I'm using RFF fields in an infinite scrolling list, and it's always slow to add new rows because it takes time to parse the new pathstrings.

I think the only longterm solution would be to accept an array like ['Channels', 'foo.bar', 'name'] in place of pathstrings like 'Channels["foo.bar"].name'.

What is the expected behavior?

We have a way to change a field that's extremely fast

Sandbox Link

What's your environment?

final-form: 4.20.4
react-final-form: 6.5.7

Other information

image

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

No branches or pull requests

1 participant