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

Preserving state through Props-turtleshell #6616

Closed
evenstensberg opened this issue Apr 25, 2016 · 7 comments
Closed

Preserving state through Props-turtleshell #6616

evenstensberg opened this issue Apr 25, 2016 · 7 comments

Comments

@evenstensberg
Copy link

evenstensberg commented Apr 25, 2016

Preserving state through Props-turtleshell

Been experimenting a bit with migrating props and state together and watch changes and how they could be used to preserve a state through multiple render passes. As addressed in #4595 , I'd like to see how this approach would turn out to be.

Props are much better to pass values than state, and how people will use this feature would be exciting to hopefully see!

To first point out some flaws that needs fix in order for this to fully function (from my perspective)

  1. A very own special method that only keeps track of state that has been called upon and not the entire tree.
  2. Loose state wrappers

I'll go in-depth later, after explaining how this would look

Fetching state and passing them as props


For now, I've just used a deep-expensive method. For stylus-sake, I've used a function as a wrapper to make it more approachable, like you can see in the picture.


skjermbilde 2016-04-25 kl 22 25 54


Now, there's a lot of sketchy things going on in the onClick attribute, but this is highly experimental for now. What is really good about this solution, is that it doesn't nest states as children, as other proposals. It rather passes your state as a prop and then updates the prop based on the previous state. ( I honestly don't have a clue why I added transferProps with this code )


And that is the intended use case here. To avoid having state as children of the parent state or as children methods. This code is also highly reusable, once perfected. For now, this is not-so-highly performance-friendly to reuse, mainly because I'm using replaceState which in itself is a expensive task. Also, it breaks, when used in the long term, like this:


giphy

skjermbilde 2016-04-25 kl 19 07 10

State to Props lifecycle functions

As mentioned, I used replaceState. Could also have used setState, but really, we should not have any of those for this feature. If this feature is a good way to keep track of state, I'd like to see the following:

  • State-to-props lifecycle

    Why? setState and replaceState should only be set on the existing state. The new prop returned would allow us to take the previous state, and ignore any other value passed by previous states. This would allow us a much faster transition from prevState to theNewProp, where we just update state as a new value of prop, rather than re-rendering the entire state tree.

  • Props-to-state lifecycle

    Why? We need some insurance that state is a valid state. Maybe as a callback function that takes the current props and allows you to set a new state value, like getInitialState with props as its arguments.

We've covered the most here. Maybe one thing to add up, is that we don't need to keep track of the entire state tree, as long as we get the previous returned state valid. This brings me up to issue 2. I'm not sure how React is set up with this right now with state wrappers, but I think it will need some fixes if this is implemented/considered.

(I tried making this as an addon without breaking encapsulation. It didn't work because I was trying to make the lifecycle functions/methods from external modules.( Yeah, I'm a terrible programming citizen 😭 ))

To clarify: For this, we need 2 new Lifecycle functions

Making sure people gets informed about this issue, @sebmarkbage @gaearon @zpao @jimfb

@jimfb
Copy link
Contributor

jimfb commented Apr 25, 2016

@ev1stensberg Thanks for the write-up!

The biggest problem with having parents maintain their child state is that: you need a way to match a component's internal state with the key of the element being generated. Once you've done this (in an automatic way), you basically end up back where we are today (with a this.state, managed by React). For this reason, I don't think we'd want to pursue this within the core until we've resolved the discussion in #4595.

I'd recommend trying to find workarounds in userland, and continue experimenting with this in a separate repository. If you really need to inject things into the core, you can probably follow an injection strategy similar to https://github.com/web-perf/react-worker-dom (although this technique is technically not supported and could break without notice).

If your library becomes popular and/or we figure out a solution in #4595, we can always reconsider. I'm going to close this out for now, since I don't think we would want to do this until we've figured out an optimal solution to #4595.

@jimfb jimfb closed this as completed Apr 25, 2016
@evenstensberg
Copy link
Author

evenstensberg commented Apr 25, 2016

Nothing about child states here @jimfb. That's what I was trying to avoid, swapping between state and props just updates the props with the current state and the opposite for props to state.

@jimfb
Copy link
Contributor

jimfb commented Apr 25, 2016

You are conflating props and state, but the concepts still exist at a higher-level. Otherwise you wouldn't need a State-to-props and props-to-state lifecycle.

@evenstensberg
Copy link
Author

Yeah, and that's the intended use. The only thing needed here is a new prop that gets updated while using props-to-state Hydrates this.state

@jimfb
Copy link
Contributor

jimfb commented Apr 25, 2016

Right, but then...

The biggest problem with having parents maintain their child state is that: you need a way to match a component's internal state with the key of the element being generated. Once you've done this (in an automatic way), you basically end up back where we are today (with a this.state, managed by React). For this reason, I don't think we'd want to pursue this within the core until we've resolved the discussion in #4595.

@evenstensberg
Copy link
Author

I don't get it... Sorry.. This "addon" -ish solution would not contain child states, but it would rather update the state based on changes to the previous. It would be similar to this.setState/replaceState. Could you fill me inn on this? Perhaps tell how it makes this.state a child rather than a parent?

@evenstensberg
Copy link
Author

Also, perhaps I got this wrong. I was thinking about state itself. Did you mean to say this.state.Something and refer to the child of state? ( If so, huge bumper for me ) Any way or another, I don't understand why internal keys to the child has anything to do with this, as you are really just updating the same state all over again...

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

2 participants