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

Feature Proposal: otherProps #205

Closed
mthadley opened this issue Feb 2, 2017 · 3 comments
Closed

Feature Proposal: otherProps #205

mthadley opened this issue Feb 2, 2017 · 3 comments

Comments

@mthadley
Copy link

mthadley commented Feb 2, 2017

Feature Proposal: otherProps

Hey everyone, so I had a feature proposal after seeing that we had a pattern that we both liked and saw ourselves repeating in probably all of our metal-jsx projects going forward. For now, I am calling it otherProps and it works something like this:

The Problem

Let's say have a basic component like Button and it is defined like this:

import Component, {Config} from 'metal-jsx';

class Button extends Component {
	render() {
		const {children, size} = this.props;

		return (
			<button class={`btn btn-size-${size}`}>
				{children}
			</button>
		);
	}
}

Button.PROPS = {
	size: Config.oneOf(['small', 'medium', 'large'])
};

export default Button;

It's pretty simple, but it allows use to create a component that behaves like a normal button, while exposing extra configuration that is specific to our app. However, we have a problem here when we want to start adding normal DOM attributes that we would expect to work with a button (or a div, input, etc.):

class Button extends Component {
	render() {
		const {children, size} = this.props;

		return (
			<button 
				class={`btn btn-size-${size}`}
				disabled={this.props.disabled}
				onClick={this.props.onClick}
				onHover={this.props.onHover}
				onMouseOver={this.props.onMouseOver}
				autofocus={this.props.autofocus}
				form={this.props.form}
				formmethod={this.props.formmethod}
				formnovalidate={this.props.formnovalidate}
				name={this.props.name}
				type={this.props.type}
			>
				{children}
			</button>
		);
	}
}

Button.PROPS = {
	size: Config.oneOf(['small', 'medium', 'large']),
	disabled: Config.bool(),
	onClick: Config.func(),
	onHover: Config.func(),
	onMouseOver: Config.func(),
	autofocus: Config.bool(),
	form: Config.string(),
	formmethod: Config.string(),
	formnovalidate: Config.bool(),
	name: Config.string(),
	type: Config.string(),
	/* ...the possibilities go on... */
};

The API of your component will essentially keep growing endlessly as our use cases change and we require the ability to add basically any of the possible attributes in the HTML standard.

The Solution

What we have instead done is use a helper function to simplify this. You can
see it copied here to metal-devtools. So the previous example becomes this:

import Component, {Config} from 'metal-jsx';
import otherProps from 'otherProps';

class Button extends Component {
	render() {
		const {children, size} = this.props;

		return (
			<button {...otherProps(this)} class={`btn btn-size-${size}`}>
				{children}
			</button>
		);
	}
}

Button.PROPS = {
	size: Config.oneOf(['small', 'medium', 'large']),
};

export default Button;

Essentially, otherProps will take the component, and from it extract any passed props that was not defined in it's PROPS configuration, and pass them to the JSXElement that it is being applied to. More specifically, it is making use of babel's JSXSpreadAttribute.

While this is useful with very basic components like this Button, (or things like an Icon component or a Card component), it can also be very useful for larger components.

Imagine you have some kind of AutoComplete component that accepts many different types of configuration through it's props. Later you write a higher-order component like SelectWithAutoComplete, that wraps your original AutoComplete component. Here you can also use otherProps, so that any prop not defined by SelectWithAutoComplete will be passed down to AutoComplete. Even nicer still, if later someone adds more to the AutoComplete API, SelectWithAutoComplete will automatically work with those new features while not requiring any changes to its code.

Implementations

My first thought was that this could just be a separate module, completely outside of metal-jsx. However because it was such a strong pattern for us, I thought it might be good to propose it here first.

  1. The simplest way would be to just expose it as a helper function like in the
    above examples.

  2. If we wanted to make it even nicer, we could check for some kind of special
    attribute that tells metal to perform this behavior:

class Button extends Component {
	render() {
		const {children, size} = this.props;

		return (
			<button delegateprops class={`btn btn-size-${size}`}>
				{children}
			</button>
		);
	}
}
  1. We could also say that the element or the child component containing the
    element will always receive the other props. Not my favorite idea, since
    you might want to instead pass props down to some nested element in our
    component. However, it would be simple.

Soy

So far this is mostly focused on metal-jsx, but it would be nice if we could also somehow implement this behavior for metal-soy as well. Maybe it is already possible to achieve similar behavior somehow through data-all but I am not sure. I know it is something we have missed when working with Soy.

Thanks!

@bryceosterhaus @ethib137

@eduardolundgren
Copy link
Contributor

The idea looks nice, even though I have few questions and implementation concerns. Let me start with basic question then we can go deeper.

When you do this:

<button {...otherProps(this)} class={`btn btn-size-${size}`}>
	{children}
</button>

Who is registering the other props as Button.PROPS in order to match to the example when you manually map them?

@mthadley
Copy link
Author

mthadley commented Feb 6, 2017

The basic implementation we have used so far can be seen here. It only takes one argument (which is the component itself, this), and it returns an object containing props. Those props will be anything not listed in the component's PROPS configuration.

So in the case of Button from above, that has a PROPS configuration like this:

Button.PROPS = {
  size: Config.oneOf(['small', 'medium', 'large'])
};

I can render it like this:

/* notice `title` and `onHover` are not listed as props above */
<Button size="small" title="myButton" onClick={myFunc} />

and calling otherProps(this) inside of Button's render will return this:

{
  title: "myButton",
  onClick: [Function]
}

From there we can just use the JSXSpreadAttribute syntax ({...obj}) to pass each of those props down to the element itself.

The reason we don't just apply something like {...this.props} directly is because we would adding attributes to the DOM element that we don't want. In the above case, that would mean we would be rendering a button to the dom with a size attribute, which doesn't make sense. We only mean to use size to derive the appropriate css class, not add it as DOM attribute.

Hopefully that helps to explain it a bit better.

@eduardolundgren
Copy link
Contributor

It does, thanks. That means you don't need the other props to be mapped as props, therefore you don't have any ability of a prop, like re-render when changed. You just want to pass them to an internal element.

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