-
Notifications
You must be signed in to change notification settings - Fork 47.7k
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
Cascading props #5943
Comments
Sounds like context :) |
Don't I have to explicitly use context variables like |
check out default props, https://facebook.github.io/react/docs/reusable-components.html#default-prop-values |
It's not the same. What I'm proposing is to make a special kind of prop which traverses down and sets all matched component props like CSS selectors do |
Ok, the use case you provided in detail is covered by default props. But you're after something bigger: You want to control child components and their props from grandparent components. This is your big picture, right?
I think what you propose is an antipattern. So let's take your example: What you are doing here requires an implicit knowledge of how the
This kind of knowledge - that, where, when, and how the You don't have these kind of problems with React as it is. Let's say your implementation looks like this: The crucial point is that the
Your API design, i.e. the interface, of your components appears to be brittle. Then again, it is really hard to get this "right"... |
@texttechne Thnx for a thoughtful answer
Actually I kind of don't want to know how it works, I just want to set some props up if they occur, likewise I don't know when I make a selector in CSS. Good example is https://github.com/callemall/material-ui (which is great), where they have to expose some guts of their components but part of them are not exposed because they use context for global theme settings. Which is kind of wacky, even though it seems reliable.
Yes, but from overall usability point of view I will likely change how grandparent calls parent and how parent handles props anyway, because Nephew props will probably differ. And it bothers me because it makes my components seem less isolated from the context they're used at especially if I tend to make them more reusable and universal, like many UI parts are. |
As I said, interface / API design is the hard part, the open challenge. In general changing the implementation shouldn't affect the interface, so you should think really hard how you design it. Furthermore, when you think this occurs quite often, challenge your assumptions about your component: Why did you need to change your interface? Have you really isolated a single component for one purpose or does it serve more than one purpose? How would the ideal API look like? ... And yes, changing the interface always hurts. But this challenge is up to you when it comes to component design, not up to the framework you use. This challenge is recurring for any API, be it a programming language, WebService or framework. Imagine you want to change the implementation of a specific language feature, but you cannot make the needed API change for backwards compatibility reasons... This revolves all around interface / API design, which isn't specific to React. So I think you should close this issue. |
I would completely agree with you if not for the fact that if people would be so good in designing their apps from scratch we would probably still use only JS+HTML with heavy OOP. Such a structure would be a better example
It's problematic to access Child properties from Grandparent, even though such a scenario is quite reccuring, especially considering heavy folding which is popular. Actually context would do the trick, if any children prop had access to it by default (and some cascading magic for more than one context in mid-children). And it even would've been a FP way. |
I agree with @texttechne:
In general, a component should be a black box. As soon as you start peaking into the component implementation, things get really messy. @esseswann said:
I understand what you're going for there, but (IMHO) this is one of the worst properties of CSS (lack of style isolation). At the very least, it's a double edged sword. Imagine that you are designing a page, and you like big checkboxes, so you double the size of all the checkboxes on the page using a css selector. But a component that used a checkbox internally was depending on the size of the checkbox to align with the size of another node. Now things are broken. You can no longer import any arbitrary components, because the components now depend on the ambient styles of the page. This is the danger / bad practice. It is an interesting discussion. Thanks for proposing it! But I'm going to close this out for the reasons @texttechne mentioned. Feel free to continue the discussion on this thread. FWIW, I think you could use context to achieve the behavior your looking for. |
@jimfb TY for a response, you gave some good explanation, but I still have some concerns
I feel a bit dumb but I don't understand how controlling props of a component from grandparent differs from controlling the same props from parent if in a corresponding use case I will have to expose those props through parent or through context anyways.
So there are two ways: make it dependant on something more abstract or add logic to a component which would make it less reusable. |
@esseswann A parent can reason about about all the possible props that it chooses to forward to its child, but it can't easily reason about all the things a grandparent might do if the grandparent is allowed to bypass the parent's API. |
A potential use case for this would be Cascading onContextMenu propUsually, a custom context menu is something you want to be global on your site - but you want children to be able to overwrite the context menu to create a more specialized one. This can't really be solved using the context api, since you have to manually hook into the context API from within the children. If you don't have direct control of the children, that isn't really an option. Component libraries often won't expose the props of their internal DOM elements, which means passing down props manually in all the components you control isn't really an option. Using a wrapper elementOf course, what you could do here is simply to wrap your entire application in a div, and then put the event listener on this div. Taking advantage of how bubbling events and stopPropagation works to make more specialized context menus. The problem with this is that it makes the DOM tree more "noisy". It adds tags that are not strictly necessary. |
@bergkvist {
"type":"div",
"key":null,
"ref":null,
"props":{
// Walk down the tree to a desired element
"children":{ // Clone the element with React.cloneElement()
"type":"div",
"key":null,
"ref":null,
"props":{
"prop":"test" // Change this prop in the vdom tree
},
"_owner":null,
"_store":{
}
}
}, // Commit the modified element back to the vdom
"_owner":null,
"_store":{
}
} This is practically monkey patching @jimfb Am I correct with this conceptually? |
After using react for a while I came up with a strange idea to support some kind of cascading props.
So we have parent component
and then a child component
and then we call Parent component with following prop
<Parent Child={{text: 'cascading prop'}}/>
It would render as this
Going further I would propse the use of CSS-like traversal selectors on these props (i.e. >, :first-child). I know all of it looks very much like some kind of mad antipattern, but I would like to hear why, besides obvious performance isssues.
The main reason behind it is that a lot of times you have to change a little bit insides of a component in one particular place and have to implement new methods\props and then declare them in parent component anyways.
The text was updated successfully, but these errors were encountered: