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

Add Mounting utility component #43

Closed
wants to merge 3 commits into from
Closed

Add Mounting utility component #43

wants to merge 3 commits into from

Conversation

TrySound
Copy link
Collaborator

No description provided.

@renatorib
Copy link
Owner

Older discussion about this: #41

@renatorib
Copy link
Owner

Personally I like the idea. But Idk if it fits the 'powerplug' concept of state containers. It's don't have any state.

@TrySound
Copy link
Collaborator Author

@renatorib Well, it has. :)

@TrySound
Copy link
Collaborator Author

@renatorib From mount to unmount it passes state.

@renatorib
Copy link
Owner

renatorib commented Jan 30, 2018

Not exactly. In your example you use Value to handle the state and not Mounting component itself. See, I'm not saying it's not useful, I really liked it. I like the idea of wrapping some lifecycles like that. But I do not see it fit into the lib, this does not even return a fn callback, it would not be possible to use with compose, for example.

@TrySound
Copy link
Collaborator Author

Okay. Let's leave it for further discussions. I'll try to fit the idea to powerplug.

@TrySound
Copy link
Collaborator Author

@renatorib I found some solution. We can extend mount/unmount callbacks with refs which are rely on them.

@TrySound
Copy link
Collaborator Author

And it's stateful. However we still need another component like Value or State, but it's like different type of state.

@renatorib
Copy link
Owner

I was thinking yesterday and I have some ideas in my head. When I mature the idea I'm going to create a discussion issue here. Meanwhile I'll leave this PR open.

@renatorib renatorib mentioned this pull request Feb 16, 2018
@TrySound
Copy link
Collaborator Author

TrySound commented Feb 16, 2018

I don't support this idea anymore. It's better to prefer lifecycle with class. Lifecycles in opposite to state containers do not introduce any functionality and may lead to many learning issues. Also 16.4 will introduce new stateful component api which will be in a way more elegant.

@TrySound TrySound closed this Feb 16, 2018
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