Should props should propagate?

[ericclemmons]

This would technically be a BC break, but I encountered a situation today where "hiding" props didn't help.

Take Stargazers.js, for example:

react-resolver/examples/stargazers/components/Stargazers.js

Lines 68 to 80 in ca5a8c9

	Stargazers.propTypes = {
	users: React.PropTypes.array.isRequired,
	};
	export default Resolver.createContainer(Stargazers, {
	resolve: {
	users: function(props) {
	const url = `https://api.github.com/repos/${props.user}/${props.repo}/stargazers`;
	return axios.get(url).then(response => response.data);
	},
	},
	});

<Stargazers /> requires a users prop, but the <StargazersContainer /> requires user and repo to fetch the data for users.

As a result, <Stargazers /> does not have access to user or repo.

This gets slightly annoying when composing multiple containers, as you pass parent props down the chain:

resolve: {
  something: (props) => props.something
}

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...

[ericclemmons]

Yea, the more I think about this, the more it doesn't seem to make sense to just pollute the base component with props.

[iamdustan]

From the consuming component, shouldn’t a given BaseComponent and BaseComponentContainer have the same interface?

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(...); },
}

[ericclemmons]

Not necessarily. As an example, a project I have lets users request information from schools.

The base component has this interface: <RequestForm info={...} form={...} />.

But, lead and form are all from stores/actions and not the concern of the parent components.

Consumers use: <RequestForm school={school} />.

React Resolver takes care of gathering existing user-info for info and generating a form from the info.

I completely agree, though, that there's an overlap in props that should allow for a short-hand.

TBH, I think your propKey: "propKey" recommendation is the most explicit and simple. I initially thought "*" would work as well, but I'd really like to recommend against blind prop passing as I've personally had lost time with bugs that were passing foo preventing resolve: { foo: ... } from ever running.

(A good example of this is having a container convert IDs => Models, e.g. <School school="1" /> => <School school={{ id: 1, name: "Foo", ... }} />.

Thoughts?

[iamdustan]

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" />

[iamdustan]

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?

[ericclemmons]

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. item) :)

[iamdustan]

Arg. I suspect I’ll need to pick this back up. I often just splat properties through for things like className or specific style properties for context and resolver is eating all of that for me at the moment.

[ericclemmons]

@iamdustan Actually, in recent projects I've found that it makes since for HoCs to compose props onto the base Component.

In other words:

• Props should propagate :)

[iamdustan]

sooooo do you have it implemented yet? :)

[AlesJiranek]

+1