Support React 16 Fragments

[dantman]

When using val+React 16 and the new fragments I get an error.

/** @jsx h */
import h from '../../app/react';
import React, {PureComponent} from 'react';

export default class MyComponent extends PureComponent {
	render() {
		return (
			<React.Fragment>
				...
			</React.Fragment>
		);
	}
}

Warning: Invalid prop `dangerouslySetInnerHTML` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.
    in React.Fragment (created by MyComponent)
    in MyComponent

When I remove the @jsx and h created by @skatejs/val it starts working again.

[treshugart]

Looks like we probably need to remove the key from the returned object here https://github.com/skatejs/val/blob/master/src/index.js#L71. Are you able to submit a PR?

[dantman]

@treshugart No. Sorry, for now I just decided not to bother trying to use val.

I don't like the requierd wrapper and @jsx comment. I'm not entirely happy with the idea of events={{}} in code when I try to make things as flat/pure optimizable as possible and hate things like {} that break that. I almost preferred the API that https://github.com/webcomponents/react-integration had. And I don't like the missing className and bool handling that react-polymer had jscissr/react-polymer#24 (comment). Not being able to just start working with val cause of this error just tipped me over the edge for now.

[treshugart]

I don't like the requierd wrapper and @jsx comment.

There's no real way around this. There' could be a separate import of import React from '@skatejs/val/react, but doing so in the main bundle would force it to also bundle React and that doesn't work if you're just using Preact.

I'm not entirely happy with the idea of events={{}} in code when I try to make things as flat/pure optimizable as possible and hate things like {} that break that.

I can respect that. I'd be interested in moving to a flatter model. You still need heuristics for events, props and attributes, though. If you have any thoughts around this, I'd be interested in hearing them. The current API was experimenting with what it would look like based on a React RFC.

And I don't like the missing className and bool handling that react-polymer had

Class name should work if it's set as a prop, which is the default. Not sure why booleans don't work, but they should.

[dantman]

I can respect that. I'd be interested in moving to a flatter model. You still need heuristics for events, props and attributes, though. If you have any thoughts around this, I'd be interested in hearing them. The current API was experimenting with what it would look like based on a React RFC.

I guess I can't really offer a suggestion for general webcomponents right now. I'm only using Polymer components, which are completely straightforward in the heuristics needed.

In the past when discussions in React/ReactDOM/JSX themselves came up about custom attributes/events I thought then that symbols/registration might be a decent way to handle them.

const onSelectedItemChanged = React.registerEvent('selected-item-changed');
// ...
<paper-dropdown-menu
  label="Your favourite pastry"
  [onSelectedItemChanged]={this.selectPastry}>
  <paper-listbox slot="dropdown-content">
    <paper-item>Croissant</paper-item>
    <paper-item>Donut</paper-item>
    <paper-item>Financier</paper-item>
    <paper-item>Madeleine</paper-item>
  </paper-listbox>
</paper-dropdown-menu>

Though even if you wanted that, you don't have the ability to update JSX to add ES2015's computed property notation as valid syntax to JSX prop names.

And I don't like the missing className and bool handling that react-polymer had

Class name should work if it's set as a prop, which is the default. Not sure why booleans don't work, but they should.

Oh I didn't see that props are set as properties on the ref instead of just left to react. I didn't see anything about it in the readme and assumed that "Everything else is just passed through and subject to the standard behaviour of whatever library you're using" meant that all other props are just given to React/Preact/etc... to keep standard behaviour.

Although know that I know that, I'm not even sure if that's actually behaviour that I want. It's really hard to decide.

[treshugart]

I guess I can't really offer a suggestion for general webcomponents right now. I'm only using Polymer components, which are completely straightforward in the heuristics needed.

Sure, but this library is for those consuming a Polymer / Skate / whatever web component element within React / Preact etc.

Although know that I know that, I'm not even sure if that's actually behaviour that I want. It's really hard to decide.

React doesn't set props which is why you need to take over the property setting for anything that isn't special to React. If you were to say <div prop={something} /> it would be set as an attribute instead. My argument is that, most of the time, you want to set props. For the edge cases, you can still set attributes.

Using the prop in element heuristic is hard, because with custom elements, you can define an element after it's been used. Anything you'd want to set as a prop (objects and such) would get set as attributes. This could just be a caveat and to force a prop set, you use a token prefix or something like prop- or $.

I'm spitballing some of the flatter structure in the v1 branch.

EDIT

I've updated it to use prop in element and the prefix, just to see what it'd look like. I'm warming up to this. I've updated the docs to reflect the new API proposal.

[dantman]

React doesn't set props which is why you need to take over the property setting for anything that isn't special to React. If you were to say

it would be set as an attribute instead. My argument is that, most of the time, you want to set props. For the edge cases, you can still set attributes.

I can mostly get on board with that. My problem is that while I dislike the idea that React suddenly switches to attributes when it sees a custom element tag, I am even more worried about the idea that all of a sudden all the props I pass are being managed through a ref and not by React, and I have no clue what kind of side effects that may have. Like having a different controlled/uncontrolled element behaviour or perhaps de-optimizing diffing react may do (e.g. not doing a dom operation if an prop hasn't changed).

I'm still waiting to see what happens in React with the discussions about properties and attributes.

Meanwhile, I guess I went ahead and created an RFC (reactjs/rfcs#28) for that [onSelectedItemChanged]={this.selectPastry} syntax. Or in my case, [obbserveProperty('selectedItem')]={this.selectPastry}.