-
Notifications
You must be signed in to change notification settings - Fork 52
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
Should props should propagate? #36
Comments
Yea, the more I think about this, the more it doesn't seem to make sense to just pollute the base component with props. |
From the consuming component, shouldn’t a given I ran into this exact snag and that props weren’t passed through caught me off guard. Also, I didn’t realize I could pass them through so easily. Could there be a declarative shorthand for passing props through on the resolve block? resolve: {
propKey: 'propKey', // automatically extract this
'*': '*', // pass all props
'...props': '...props',
promise() { return fetch(...); },
} |
Not necessarily. As an example, a project I have lets users request information from schools. The base component has this interface: But, Consumers use: React Resolver takes care of gathering existing user-info for I completely agree, though, that there's an overlap in props that should allow for a short-hand. TBH, I think your (A good example of this is having a container convert IDs => Models, e.g. Thoughts? |
Optionally, could the container automatically pass through declared propTypes?An incomplete idea: class Base extends Component { render() { return <div /> }
Base.propTypes = {
school: PropTypes.shape().isRequired,
principal: PropTypes.string(),
};
export default Resolver.createContainer({
resolve(props) {
return Store.getSchool({id: props.id});
}
});
<Base id={1234} />
<Base school={{name: 'Huff High', grades: '9-12'}} principal="Mr. Wilson" /> |
I guess part of my thinking is should a consuming component be aware of it’s child being automatically, asynchronously resolved or is that a private implementation detail? |
Intersting! I don't see why the container can't be automatically aware, since it's meant to be smart. I just ran into specific issues when names are more ambiguous (e.g. |
Arg. I suspect I’ll need to pick this back up. I often just splat properties through for things like |
@iamdustan Actually, in recent projects I've found that it makes since for HoCs to compose props onto the base Component. In other words:
|
sooooo do you have it implemented yet? :) |
1 |
* iamdustan-props-passthru: 1.2.1 Propagate props through to Container child. Fixes #36
* v1: Remove iojs from Travis Update changelog 1.2.2 1.2.1 1.2.0 Rebuild for #56 Fixed values and props not being passed down to the target component Add version documentation Propagate props through to Container child. Fixes #36 Adds decorators Init documentation for the third argument for `Resolve.render`. Conflicts: dist/Container.js dist/Resolver.js dist/ResolverError.js dist/index.js examples/stargazers/components/Stargazers.js examples/stargazers/package.json examples/stargazers/public/main.min.js src/Container.js src/Resolver.js
This would technically be a BC break, but I encountered a situation today where "hiding" props didn't help.
Take
Stargazers.js
, for example:<Stargazers />
requires ausers
prop, but the<StargazersContainer />
requiresuser
andrepo
to fetch the data forusers
.As a result,
<Stargazers />
does not have access touser
orrepo
.This gets slightly annoying when composing multiple containers, as you pass parent props down the chain:
My gut says that this mild inconvenience can be solved through another means, and that the base component shouldn't expect parent props to be leaked through...
The text was updated successfully, but these errors were encountered: