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

RFC: Plan for custom element attributes/properties in React 17 #11347

Open
robdodson opened this Issue Oct 24, 2017 · 67 comments

Comments

Projects
None yet
@robdodson

robdodson commented Oct 24, 2017

This is meant to address #7249. The doc outlines the pros and cons of various approaches React could use to handle attributes and properties on custom elements.

TOC/Summary

  • Background
  • Proposals
    • Option 1: Only set properties
      • Pros
        • Easy to understand/implement
        • Avoids conflict with future global attributes
        • Takes advantage of custom element "upgrade"
        • Custom elements treated like any other React component
      • Cons
        • Possibly a breaking change
        • Need ref to set attribute
        • Not clear how server-side rendering would work
    • Option 2: Properties-if-available
      • Pros
        • Non-breaking change
      • Cons
        • Developers need to understand the heuristic
        • Falling back to attributes may conflict with future globals
    • Option 3: Differentiate properties with a sigil
      • Pros
        • Non-breaking change that developers can opt-in to
        • Similar to how other libraries handle attributes/properties
        • The system is explicit
      • Cons
        • It’s new syntax
        • Not clear how server-side rendering would work
    • Option 4: Add an attributes object
      • Pros
        • The system is explicit
        • Extending syntax may also solve issues with event handling
      • Cons
        • It’s new syntax
        • It may be a breaking change
        • It may be a larger change than any of the previous proposals
    • Option 5: An API for consuming custom elements
      • Pros
        • The system is explicit
        • Non-breaking change
        • Idiomatic to React
      • Cons
        • Could be a lot of work for a complex component
        • May bloat bundle size
        • Config needs to keep pace with the component

Background

When React tries to pass data to a custom element it always does so using HTML attributes.

<x-foo bar={baz}> // same as setAttribute('bar', baz)

Because attributes must be serialized to strings, this approach creates problems when the data being passed is an object or array. In that scenario, we end up with something like:

<x-foo bar="[object Object]">

The workaround for this is to use a ref to manually set the property.

<x-foo ref={el => el.bar = baz}>

This workaround feels a bit unnecessary as the majority of custom elements being shipped today are written with libraries which automatically generate JavaScript properties that back all of their exposed attributes. And anyone hand-authoring a vanilla custom element is encouraged to follow this practice as well. We'd like to ideally see runtime communication with custom elements in React use JavaScript properties by default.

This doc outlines a few proposals for how React could be updated to make this happen.

Proposals

Option 1: Only set properties

Rather than try to decide if a property or attribute should be set, React could always set properties on custom elements. React would NOT check to see if the property exists on the element beforehand.

Example:

<x-foo bar={baz}>

The above code would result in React setting the .bar property of the x-foo element equal to the value of baz.

For camelCased property names, React could use the same style it uses today for properties like tabIndex.

<x-foo squidInk={pasta}> // sets .squidInk = pasta

Pros

Easy to understand/implement

This model is simple, explicit, and dovetails with React’s "JavaScript-centric API to the DOM".

Any element created with libraries like Polymer or Skate will automatically generate properties to back their exposed attributes. These elements should all "just work" with the above approach. Developers hand-authoring vanilla components are encouraged to back attributes with properties as that mirrors how modern (i.e. not oddballs like <input>) HTML5 elements (<video>, <audio>, etc.) have been implemented.

Avoids conflict with future global attributes

When React sets an attribute on a custom element there’s always the risk that a future version of HTML will ship a similarly named attribute and break things. This concern was discussed with spec authors but there is no clear solution to the problem. Avoiding attributes entirely (except when a developer explicitly sets one using ref) may sidestep this issue until the browsers come up with a better solution.

Takes advantage of custom element "upgrade"

Custom elements can be lazily upgraded on the page and some PRPL patterns rely on this technique. During the upgrade process, a custom element can access the properties passed to it by React—even if those properties were set before the definition loaded—and use them to render initial state.

Custom elements treated like any other React component

When React components pass data to one another they already use properties. This would just make custom elements behave the same way.

Cons

Possibly a breaking change

If a developer has been hand-authoring vanilla custom elements which only have an attributes API, then they will need to update their code or their app will break. The fix would be to use a ref to set the attribute (explained below).

Need ref to set attribute

By changing the behavior so properties are preferred, it means developers will need to use a ref in order to explicitly set an attribute on a custom element.

<custom-element ref={el => el.setAttribute('my-attr', val)} />

This is just a reversal of the current behavior where developers need a ref in order to set a property. Since developers should rarely need to set attributes on custom elements, this seems like a reasonable trade-off.

Not clear how server-side rendering would work

It's not clear how this model would map to server-side rendering custom elements. React could assume that the properties map to similarly named attributes and attempt to set those on the server, but this is far from bulletproof and would possibly require a heuristic for things like camelCased properties -> dash-cased attributes.

Option 2: Properties-if-available

At runtime React could attempt to detect if a property is present on a custom element. If the property is present React will use it, otherwise it will fallback to setting an attribute. This is the model Preact uses to deal with custom elements.

Pseudocode implementation:

if (propName in element) {
  element[propName] = value;
} else {
  element.setAttribute(propName.toLowerCase(), value);
}

Possible steps:

  • If an element has a defined property, React will use it.

  • If an element has an undefined property, and React is trying to pass it primitive data (string/number/boolean), it will use an attribute.

    • Alternative: Warn and don’t set.
  • If an element has an undefined property, and React is trying to pass it an object/array it will set it as a property. This is because some-attr="[object Object]” is not useful.

    • Alternative: Warn and don’t set.
  • If the element is being rendered on the server, and React is trying to pass it a string/number/boolean, it will use an attribute.

  • If the element is being rendered on the server, and React is trying to pass it a object/array, it will not do anything.

Pros

Non-breaking change

It is possible to create a custom element that only uses attributes as its interface. This authoring style is NOT encouraged, but it may happen regardless. If a custom element author is relying on this behavior then this change would be non-breaking for them.

Cons

Developers need to understand the heuristic

Developers might be confused when React sets an attribute instead of a property depending on how they’ve chosen to load their element.

Falling back to attributes may conflict with future globals

Sebastian raised a concern that using in to check for the existence of a property on a custom element might accidentally detect a property on the superclass (HTMLElement).

There are also other potential conflicts with global attributes discussed previously in this doc.

Option 3: Differentiate properties with a sigil

React could continue setting attributes on custom elements, but provide a sigil that developers could use to explicitly set properties instead. This is similar to the approach used by Glimmer.js.

Glimmer example:

<custom-img @src="corgi.jpg" @hiResSrc="corgi@2x.jpg" width="100%">

In the above example, the @ sigil indicates that src and hiResSrc should pass data to the custom element using properties, and width should be serialized to an attribute string.

Because React components already pass data to one another using properties, there would be no need for them to use the sigil (although it would work if they did, it would just be redundant). Instead, it would primarily be used as an explicit instruction to pass data to a custom element using JavaScript properties.

h/t to @developit of Preact for suggesting this approach :)

Pros

Non-breaking change that developers can opt-in to

All pre-existing React + custom element apps would continue to work exactly as they have. Developers could choose if they wanted to update their code to use the new sigil style.

Similar to how other libraries handle attributes/properties

Similar to Glimmer, both Angular and Vue use modifiers to differentiate between attributes and properties.

Vue example:

<!-- Vue will serialize `foo` to an attribute string, and set `squid` using a JavaScript property -->
<custom-element :foo="bar” :squid.prop=”ink”>

Angular example:

<!-- Angular will serialize `foo` to an attribute string, and set `squid` using a JavaScript property -->
<custom-element [attr.foo]="bar” [squid]=”ink”>

The system is explicit

Developers can tell React exactly what they want instead of relying on a heuristic like the properties-if-available approach.

Cons

It’s new syntax

Developers need to be taught how to use it and it needs to be thoroughly tested to make sure it is backwards compatible.

Not clear how server-side rendering would work

Should the sigil switch to using a similarly named attribute?

Option 4: Add an attributes object

React could add additional syntax which lets authors explicitly pass data as attributes. If developers do not use this attributes object, then their data will be passed using JavaScript properties.

Example:

const bar = 'baz';
const hello = 'World';
const width = '100%';
const ReactElement = <Test
  foo={bar} // uses JavaScript property
  attrs={{ hello, width }} // serialized to attributes
/>;

This idea was originally proposed by @treshugart, author of Skate.js, and is implemented in the val library.

Pros

The system is explicit

Developers can tell React exactly what they want instead of relying on a heuristic like the properties-if-available approach.

Extending syntax may also solve issues with event handling

Note: This is outside the scope of this document but maybe worth mentioning :)

Issue #7901 requests that React bypass its synthetic event system when declarative event handlers are added to custom elements. Because custom element event names are arbitrary strings, it means they can be capitalized in any fashion. To bypass the synthetic event system today will also mean needing to come up with a heuristic for mapping event names from JSX to addEventListener.

// should this listen for: 'foobar', 'FooBar', or 'fooBar'?
onFooBar={handleFooBar}

However, if the syntax is extended to allow attributes it could also be extended to allow events as well:

const bar = 'baz';
const hello = 'World';
const SquidChanged = e => console.log('yo');
const ReactElement = <Test
  foo={bar}
  attrs={{ hello }}
  events={{ SquidChanged}} // addEventListener('SquidChanged', …)
/>;

In this model the variable name is used as the event name. No heuristic is needed.

Cons

It’s new syntax

Developers need to be taught how to use it and it needs to be thoroughly tested to make sure it is backwards compatible.

It may be a breaking change

If any components already rely on properties named attrs or events, it could break them.

It may be a larger change than any of the previous proposals

For React 17 it may be easier to make an incremental change (like one of the previous proposals) and position this proposal as something to take under consideration for a later, bigger refactor.

Option 5: An API for consuming custom elements

This proposal was offered by @sophiebits and @gaearon from the React team

React could create a new API for consuming custom elements that maps the element’s behavior with a configuration object.

Pseudocode example:

const XFoo = ReactDOM.createCustomElementType({
  element: ‘x-foo’,
  ‘my-attr’: // something that tells React what to do with it
  someRichDataProp: // something that tells React what to do with it
});

The above code returns a proxy component, XFoo that knows how to pass data to a custom element depending on the configuration you provide. You would use this proxy component in your app instead of using the custom element directly.

Example usage:

<XFoo someRichDataProp={...} />

Pros

The system is explicit

Developers can tell React the exact behavior they want.

Non-breaking change

Developers can opt-in to using the object or continue using the current system.

Idiomatic to React

This change doesn’t require new JSX syntax, and feels more like other APIs in React. For example, PropTypes (even though it’s being moved into its own package) has a somewhat similar approach.

Cons

Could be a lot of work for a complex component

Polymer’s paper-input element has 37 properties, so it would produce a very large config. If developers are using a lot of custom elements in their app, that may equal a lot of configs they need to write.

May bloat bundle size

Related to the above point, each custom element class now incurs the cost of its definition + its config object size.

Note: I'm not 100% sure if this is true. Someone more familiar with the React build process could verify.

Config needs to keep pace with the component

Every time the component does a minor version revision that adds a new property, the config will need to be updated as well. That’s not difficult, but it does add maintenance. Maybe if configs are generated from source this is less of a burden, but that may mean needing to create a new tool to generate configs for each web component library.

cc @sebmarkbage @gaearon @developit @treshugart @justinfagnani

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Oct 24, 2017

Apologies for the long read, but I wanted to make sure I was thoroughly exploring each option. I don't want to bias things too much with my own opinion, but if I were in a position to choose, I think I'd go with option 3.

Option 3 is backwards compatible, declarative, and explicit. There’s no need to maintain a fallback heuristic, and other libraries already provide similar sigils/modifiers.

robdodson commented Oct 24, 2017

Apologies for the long read, but I wanted to make sure I was thoroughly exploring each option. I don't want to bias things too much with my own opinion, but if I were in a position to choose, I think I'd go with option 3.

Option 3 is backwards compatible, declarative, and explicit. There’s no need to maintain a fallback heuristic, and other libraries already provide similar sigils/modifiers.

@worawit15379

This comment has been minimized.

Show comment
Hide comment
@worawit15379

worawit15379 Oct 24, 2017

worawit15379 commented Oct 24, 2017

@jeremenichelli

This comment has been minimized.

Show comment
Hide comment
@jeremenichelli

jeremenichelli Oct 24, 2017

Contributor

I'm between option 2 and option 3, I think that React has handled behavior and API changes very well in the past. Introducting warnings and links to docs might serve well to help developers understand what's happening under the hood.

Option 3 looks attractive because of its declarative nature, while reading JSX code new coming developers will know immediately what React will do when rendering the element.

Contributor

jeremenichelli commented Oct 24, 2017

I'm between option 2 and option 3, I think that React has handled behavior and API changes very well in the past. Introducting warnings and links to docs might serve well to help developers understand what's happening under the hood.

Option 3 looks attractive because of its declarative nature, while reading JSX code new coming developers will know immediately what React will do when rendering the element.

@cjorasch

This comment has been minimized.

Show comment
Hide comment
@cjorasch

cjorasch Oct 24, 2017

Comments on option 2

Developers might be confused when React sets an attribute instead of a property depending on how they’ve chosen to load their element.

Do consumers of a custom element need to understand this distinction? Or is that only important to the author of the custom element? It seems like the author of the element will need to handle attributes for anything used in HTML (since that is the only way data gets passed from HTML usage) and properties if they want to support complex values or property get/set from DOM. It is even possible an author could have something initially implemented as an attribute and then later add a property with the same name to support more flexible data types and still back the property with a value stored in the attributes.

Naming collisions with future HTMLElement attributes and properties seems like a weakness in the Web Components standards in general since that can lead to errors regardless of the binding approach.

If an element has an undefined property, and React is trying to pass it an object/array it will set it as a property. This is because some-attr="[object Object]” is not useful.

It seems confusing to bind differently based on the value. If the author of the element has not specified a property getter/setter to handle the value then setting the property would cause the element to behave like the value was never specified which might be harder to debug.

Comments on option 3

Another potential con with option 3 is that it requires the consumer of the custom element to know whether the element has implemented something as a property or as an attribute. If you are using a mix of React components and custom elements it could be confusing to set React props using one syntax and custom element properties using a different syntax.

cjorasch commented Oct 24, 2017

Comments on option 2

Developers might be confused when React sets an attribute instead of a property depending on how they’ve chosen to load their element.

Do consumers of a custom element need to understand this distinction? Or is that only important to the author of the custom element? It seems like the author of the element will need to handle attributes for anything used in HTML (since that is the only way data gets passed from HTML usage) and properties if they want to support complex values or property get/set from DOM. It is even possible an author could have something initially implemented as an attribute and then later add a property with the same name to support more flexible data types and still back the property with a value stored in the attributes.

Naming collisions with future HTMLElement attributes and properties seems like a weakness in the Web Components standards in general since that can lead to errors regardless of the binding approach.

If an element has an undefined property, and React is trying to pass it an object/array it will set it as a property. This is because some-attr="[object Object]” is not useful.

It seems confusing to bind differently based on the value. If the author of the element has not specified a property getter/setter to handle the value then setting the property would cause the element to behave like the value was never specified which might be harder to debug.

Comments on option 3

Another potential con with option 3 is that it requires the consumer of the custom element to know whether the element has implemented something as a property or as an attribute. If you are using a mix of React components and custom elements it could be confusing to set React props using one syntax and custom element properties using a different syntax.

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Oct 24, 2017

Do consumers of a custom element need to understand this distinction? Or is that only important to the author of the custom element?

I doubt it's actually a huge issue because, as you pointed out, the element author should define an attribute and property for the underlying value and accept data from both. I would also add that they should keep the attribute and property in sync (so setting one sets the other).

Naming collisions with future HTMLElement attributes and properties seems like a weakness in the Web Components standards in general since that can lead to errors regardless of the binding approach.

I agree but I'm not sure if this is something React needs to try to work around in their library. It feels like a problem that needs to be solved as part of the custom elements spec. I can see if we can discuss it as part of the upcoming TPAC standards meeting.

I should add, for properties this isn't as bad because the element-defined property will shadow the future property added to HTMLElement. So if you were passing data to a custom element as a js property, it would continue to work. The main issue seems to be around attributes since they are global.

It seems confusing to bind differently based on the value. If the author of the element has not specified a property getter/setter to handle the value then setting the property would cause the element to behave like the value was never specified which might be harder to debug.

In the case where a custom element is lazy loaded and "upgraded", it will initially have undefined properties. This addresses that use case by making sure those elements still receive their data and they can use it post-upgrade.

It's true that if the author doesn't define a getter/setter for a value this would not be very useful. But it's also not useful to have an my-attr=[object Object]. And since you don't know if the property is truly undefined or if they definition is just being lazy loaded, it seems safest to set the property.

Another potential con with option 3 is that it requires the consumer of the custom element to know whether the element has implemented something as a property or as an attribute.

I think you're essentially in the same boat today because there's nothing that forces a custom element author to define an attribute instead of a property. So I could have an element with a properties only API that would not receive any data from React's current system and I would need to know to use ref to directly set the js properties.

Because custom elements are meant as a primitive, there's nothing that enforces creating corresponding attributes and properties. But we're trying very hard to encourage doing so as a best practice, and all of the libraries that I know of today create backing properties for their attributes.

[edit]

As you stated in your earlier point:

It seems like the author of the element will need to handle attributes for anything used in HTML (since that is the only way data gets passed from HTML usage) and properties if they want to support complex values or property get/set from DOM.

Because you never know how a user will try to pass data to your element, you end up needing to have attribute-property correspondence anyway. I imagine if option 3 shipped that most folks would just bind everything using the @ sigil because it'd be easiest. That's how I work with custom elements in Vue today since they expose a .prop modifier.

robdodson commented Oct 24, 2017

Do consumers of a custom element need to understand this distinction? Or is that only important to the author of the custom element?

I doubt it's actually a huge issue because, as you pointed out, the element author should define an attribute and property for the underlying value and accept data from both. I would also add that they should keep the attribute and property in sync (so setting one sets the other).

Naming collisions with future HTMLElement attributes and properties seems like a weakness in the Web Components standards in general since that can lead to errors regardless of the binding approach.

I agree but I'm not sure if this is something React needs to try to work around in their library. It feels like a problem that needs to be solved as part of the custom elements spec. I can see if we can discuss it as part of the upcoming TPAC standards meeting.

I should add, for properties this isn't as bad because the element-defined property will shadow the future property added to HTMLElement. So if you were passing data to a custom element as a js property, it would continue to work. The main issue seems to be around attributes since they are global.

It seems confusing to bind differently based on the value. If the author of the element has not specified a property getter/setter to handle the value then setting the property would cause the element to behave like the value was never specified which might be harder to debug.

In the case where a custom element is lazy loaded and "upgraded", it will initially have undefined properties. This addresses that use case by making sure those elements still receive their data and they can use it post-upgrade.

It's true that if the author doesn't define a getter/setter for a value this would not be very useful. But it's also not useful to have an my-attr=[object Object]. And since you don't know if the property is truly undefined or if they definition is just being lazy loaded, it seems safest to set the property.

Another potential con with option 3 is that it requires the consumer of the custom element to know whether the element has implemented something as a property or as an attribute.

I think you're essentially in the same boat today because there's nothing that forces a custom element author to define an attribute instead of a property. So I could have an element with a properties only API that would not receive any data from React's current system and I would need to know to use ref to directly set the js properties.

Because custom elements are meant as a primitive, there's nothing that enforces creating corresponding attributes and properties. But we're trying very hard to encourage doing so as a best practice, and all of the libraries that I know of today create backing properties for their attributes.

[edit]

As you stated in your earlier point:

It seems like the author of the element will need to handle attributes for anything used in HTML (since that is the only way data gets passed from HTML usage) and properties if they want to support complex values or property get/set from DOM.

Because you never know how a user will try to pass data to your element, you end up needing to have attribute-property correspondence anyway. I imagine if option 3 shipped that most folks would just bind everything using the @ sigil because it'd be easiest. That's how I work with custom elements in Vue today since they expose a .prop modifier.

@jeremenichelli

This comment has been minimized.

Show comment
Hide comment
@jeremenichelli

jeremenichelli Oct 24, 2017

Contributor

it requires the consumer of the custom element to know whether the element has implemented something as a property or as an attribute

That's not something React should worry as Rob said in my opinion, it's the custom element author's responsability to inform the user how the element works.

And it's actually the way that we need to do it today, for example think about the <video> element, let's say you need to mute it or change the current time inside a component.

muted works as a boolean attribute

render() {
  return (
    <div className="video--wrapper">
      <video muted={ this.state.muted } />
    </div>
  );
}

For the current time you need to create a ref pointing to the video element and change the property.

render() {
  return (
    <div className="video--wrapper">
      <video ref={ el => this.video = el } muted={ this.state.muted } />
    </div>
  );
}

Then create an event handler, an instance method and manually set the property to the DOM element.

onCurrenTimeChange(e) {
  this.video.currentTime = e.value;
}

If you think about it it kinda breaks the declarative model React itself imposes with its API and JSX abstract layer since the currentTime it's clearly a state in the wrapper component, with property binding we would still need the event handler but the JSX abstraction model would be more declarative and refs wouldn't be necessary just for this:

render() {
  return (
    <div className="video--wrapper">
      <video muted={ this.state.muted } @currentTime={ this.state.currentTime } />
    </div>
  );
}

My point is that whether you are relying on native or custom elements, you still need to know your way around them based on documentation, the difference that in the second case it should come from the custom element's author.

@cjorasch my two cents :)

Contributor

jeremenichelli commented Oct 24, 2017

it requires the consumer of the custom element to know whether the element has implemented something as a property or as an attribute

That's not something React should worry as Rob said in my opinion, it's the custom element author's responsability to inform the user how the element works.

And it's actually the way that we need to do it today, for example think about the <video> element, let's say you need to mute it or change the current time inside a component.

muted works as a boolean attribute

render() {
  return (
    <div className="video--wrapper">
      <video muted={ this.state.muted } />
    </div>
  );
}

For the current time you need to create a ref pointing to the video element and change the property.

render() {
  return (
    <div className="video--wrapper">
      <video ref={ el => this.video = el } muted={ this.state.muted } />
    </div>
  );
}

Then create an event handler, an instance method and manually set the property to the DOM element.

onCurrenTimeChange(e) {
  this.video.currentTime = e.value;
}

If you think about it it kinda breaks the declarative model React itself imposes with its API and JSX abstract layer since the currentTime it's clearly a state in the wrapper component, with property binding we would still need the event handler but the JSX abstraction model would be more declarative and refs wouldn't be necessary just for this:

render() {
  return (
    <div className="video--wrapper">
      <video muted={ this.state.muted } @currentTime={ this.state.currentTime } />
    </div>
  );
}

My point is that whether you are relying on native or custom elements, you still need to know your way around them based on documentation, the difference that in the second case it should come from the custom element's author.

@cjorasch my two cents :)

@effulgentsia

This comment has been minimized.

Show comment
Hide comment
@effulgentsia

effulgentsia Oct 24, 2017

If we were designing this from scratch, without needing to consider backwards compatibility, I think option 1 would be the most idiomatic per React’s "JavaScript-centric API to the DOM".

With regard to server-side rendering, could that problem be solved by providing an API for application code to inform React on how to map custom element properties to attributes? Similar to the maps that React already maintains for platform-defined attributes? This API would only need to be invoked once per custom element name (not for each instance of it), and only for properties that don't follow a straight 1:1 correspondence with their attribute, which should hopefully be relatively rare.

If we're concerned about this being too much of a breaking change though, then I think option 3 is pretty appealing as well. If the sigil signifies a property, I would suggest ".", since that's already JavaScript's property accessor. However, I think it's unfortunate to make every instance of where a custom element is used in an application be responsible for knowing when to use an attribute vs. when to use a property. What I prefer about option 1 is that even if a property to attribute map is needed, that mapping code can be isolated from all the JSX usages.

effulgentsia commented Oct 24, 2017

If we were designing this from scratch, without needing to consider backwards compatibility, I think option 1 would be the most idiomatic per React’s "JavaScript-centric API to the DOM".

With regard to server-side rendering, could that problem be solved by providing an API for application code to inform React on how to map custom element properties to attributes? Similar to the maps that React already maintains for platform-defined attributes? This API would only need to be invoked once per custom element name (not for each instance of it), and only for properties that don't follow a straight 1:1 correspondence with their attribute, which should hopefully be relatively rare.

If we're concerned about this being too much of a breaking change though, then I think option 3 is pretty appealing as well. If the sigil signifies a property, I would suggest ".", since that's already JavaScript's property accessor. However, I think it's unfortunate to make every instance of where a custom element is used in an application be responsible for knowing when to use an attribute vs. when to use a property. What I prefer about option 1 is that even if a property to attribute map is needed, that mapping code can be isolated from all the JSX usages.

@cjorasch

This comment has been minimized.

Show comment
Hide comment
@cjorasch

cjorasch Oct 24, 2017

In the case where a custom element is lazy loaded and "upgraded", it will initially have undefined properties. This addresses that use case by making sure those elements still receive their data and they can use it post-upgrade.

Maybe I don't understand the upgrade process. Elements would typically have properties defined as getters/setters in the class prototype. Checking propName in element would return true because of the existence of the getter/setter even if the property value was still undefined. During upgrade do property values get set on some temporary instance and then later get copied to the actual instance once the lazy load is complete?

cjorasch commented Oct 24, 2017

In the case where a custom element is lazy loaded and "upgraded", it will initially have undefined properties. This addresses that use case by making sure those elements still receive their data and they can use it post-upgrade.

Maybe I don't understand the upgrade process. Elements would typically have properties defined as getters/setters in the class prototype. Checking propName in element would return true because of the existence of the getter/setter even if the property value was still undefined. During upgrade do property values get set on some temporary instance and then later get copied to the actual instance once the lazy load is complete?

@effulgentsia

This comment has been minimized.

Show comment
Hide comment
@effulgentsia

effulgentsia Oct 24, 2017

Upgrading is the process by which the custom element receives its class. Prior to that, it's not an instance of that class, so the property getters/setters aren't available.

effulgentsia commented Oct 24, 2017

Upgrading is the process by which the custom element receives its class. Prior to that, it's not an instance of that class, so the property getters/setters aren't available.

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Oct 24, 2017

@jeremenichelli

muted works as a boolean attribute

just checked and it also has a corresponding property though it doesn't seem to be documented on MDN :P

For the current time you need to create a ref pointing to the video element and change the property.

Yeah occasionally you'll encounter properties-only APIs on modern HTML elements. currentTime updates at a high frequency so it wouldn't make sense to reflect it to an HTML attribute.

My point is that wether you are relying on native or custom elements, you still need to know your way around them based on documentation

Yep there's unfortunately no one-size-fits-all attributes/properties rule. But I think generally speaking you can lean heavily on properties and provide syntax so developers can use attributes in special cases.

robdodson commented Oct 24, 2017

@jeremenichelli

muted works as a boolean attribute

just checked and it also has a corresponding property though it doesn't seem to be documented on MDN :P

For the current time you need to create a ref pointing to the video element and change the property.

Yeah occasionally you'll encounter properties-only APIs on modern HTML elements. currentTime updates at a high frequency so it wouldn't make sense to reflect it to an HTML attribute.

My point is that wether you are relying on native or custom elements, you still need to know your way around them based on documentation

Yep there's unfortunately no one-size-fits-all attributes/properties rule. But I think generally speaking you can lean heavily on properties and provide syntax so developers can use attributes in special cases.

@jeremenichelli

This comment has been minimized.

Show comment
Hide comment
@jeremenichelli

jeremenichelli Oct 24, 2017

Contributor

@robdodson yeap, I knew about the muted property too 😄 I just used these two to prove that already in the wild there isn't a one-size-fits-all rule as you mentioned.

We will have to rely on documentation on both native and custom elements, so it's something I wouldn't mind for this decision.

While writing the last code snippet I kinda liked the property binding though 💟

Contributor

jeremenichelli commented Oct 24, 2017

@robdodson yeap, I knew about the muted property too 😄 I just used these two to prove that already in the wild there isn't a one-size-fits-all rule as you mentioned.

We will have to rely on documentation on both native and custom elements, so it's something I wouldn't mind for this decision.

While writing the last code snippet I kinda liked the property binding though 💟

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Oct 24, 2017

@effulgentsia

However, I think it's unfortunate to make every instance of where a custom element is used in an application be responsible for knowing when to use an attribute vs. when to use a property.

I think this is already the case today though. Since the major custom element libraries (polymer, skate, possibly others?) automatically create backing properties for all exposed attributes, developers could just use the sigil for every property on a custom element. It would probably be a rare occurrence for them to need to switch to using an attribute.

@cjorasch

RE: upgrade. As @effulgentsia mentioned, it's possible to have a custom element on the page but load its definition at a later time. <x-foo> will initially be an instance of HTMLElement and when I load its definition later it "upgrades" and becomes an instance of the XFoo class. At this point all of its lifecycle callbacks get executed. We use this technique in the Polymer Starter Kit project. Kind of like this:

<app-router>
  <my-view1></my-view1>
  <my-view2></my-view2>
</app-router>

In the above example, we won't load the definition for my-view2 until the router changes to it.

It's entirely possible to set a property on the element before it has upgraded, and once the definition is loaded the element can grab that data during one of its lifecycle callbacks.

robdodson commented Oct 24, 2017

@effulgentsia

However, I think it's unfortunate to make every instance of where a custom element is used in an application be responsible for knowing when to use an attribute vs. when to use a property.

I think this is already the case today though. Since the major custom element libraries (polymer, skate, possibly others?) automatically create backing properties for all exposed attributes, developers could just use the sigil for every property on a custom element. It would probably be a rare occurrence for them to need to switch to using an attribute.

@cjorasch

RE: upgrade. As @effulgentsia mentioned, it's possible to have a custom element on the page but load its definition at a later time. <x-foo> will initially be an instance of HTMLElement and when I load its definition later it "upgrades" and becomes an instance of the XFoo class. At this point all of its lifecycle callbacks get executed. We use this technique in the Polymer Starter Kit project. Kind of like this:

<app-router>
  <my-view1></my-view1>
  <my-view2></my-view2>
</app-router>

In the above example, we won't load the definition for my-view2 until the router changes to it.

It's entirely possible to set a property on the element before it has upgraded, and once the definition is loaded the element can grab that data during one of its lifecycle callbacks.

@effulgentsia

This comment has been minimized.

Show comment
Hide comment
@effulgentsia

effulgentsia Oct 24, 2017

developers could just use the sigil for every property on a custom element

If developers started doing that, then how would that differentiate using a property because you "can" from using a property because you "must"? And isn't that a differentiation that's needed for server-side rendering?

effulgentsia commented Oct 24, 2017

developers could just use the sigil for every property on a custom element

If developers started doing that, then how would that differentiate using a property because you "can" from using a property because you "must"? And isn't that a differentiation that's needed for server-side rendering?

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Oct 24, 2017

If developers started doing that, then how would that differentiate using a property because you "can" from using a property because you "must"?

Sorry, maybe I phrased that wrong. I meant that developers would likely use the sigil because it would give the most consistent result. You can use it to pass primitive data or rich data like objects and arrays and it'll always work. I think working with properties at runtime is generally preferred to working with attributes since attributes tend to be used more for initial configuration.

And isn't that a differentiation that's needed for server-side rendering?

It might be the case that on the server the sigil would fallback to setting an attribute.

robdodson commented Oct 24, 2017

If developers started doing that, then how would that differentiate using a property because you "can" from using a property because you "must"?

Sorry, maybe I phrased that wrong. I meant that developers would likely use the sigil because it would give the most consistent result. You can use it to pass primitive data or rich data like objects and arrays and it'll always work. I think working with properties at runtime is generally preferred to working with attributes since attributes tend to be used more for initial configuration.

And isn't that a differentiation that's needed for server-side rendering?

It might be the case that on the server the sigil would fallback to setting an attribute.

@effulgentsia

This comment has been minimized.

Show comment
Hide comment
@effulgentsia

effulgentsia Oct 24, 2017

It might be the case that on the server the sigil would fallback to setting an attribute.

I don't think that would work if the reason for the sigil is that it's a property that doesn't exist as an attribute, such as video's currentTime.

differentiate using a property because you "can" from using a property because you "must"

I think this differentiation is important, because there's entirely different reasons for choosing to use an attribute or property as an optimization (e.g., SSR preferring attributes vs. client-side rendering preferring properties) vs. something that exists either as only an attribute or only a property.

With regard to server-side rendering, could that problem be solved by providing an API for application code to inform React on how to map custom element properties to attributes?

To be more specific, I'm suggesting something like this:

ReactDOM.defineCustomElementProp(elementName, propName, domPropertyName, htmlAttributeName, attributeSerializer)

Examples:

// 'muted' can be set as either a property or an attribute.
ReactDOM.defineCustomElementProp('x-foo', 'muted', 'muted', 'muted')

// 'currentTime' can only be set as a property.
ReactDOM.defineCustomElementProp('x-foo', 'currentTime', 'currentTime', null)

// 'my-attribute' can only be set as an attribute.
ReactDOM.defineCustomElementProp('x-foo', 'my-attribute', null, 'my-attribute')

// 'richData' can be set as either a property or an attribute.
// When setting as an attribute, set it as a JSON string rather than "[object Object]".
ReactDOM.defineCustomElementProp('x-foo', 'richData', 'richData', 'richdata', JSON.stringify)

For something that can only be a property (where htmlAttributeName is null), SSR would skip over rendering it and then hydrate it on the client.

For something that can only be an attribute (where domPropertyName is null), React would invoke setAttribute() as currently in v16.

For something that can be both, React could choose whatever strategy is most optimal. Perhaps that means always setting as a property on client-side, but as an attribute server-side. Perhaps it means setting as an attribute when initially creating the element, but setting as a property when later patching from the vdom. Perhaps it means only setting as an attribute when the value is a primitive type. Ideally, React should be able to change the strategy whenever it wants to as an internal implementation detail.

When React encounters a prop for which defineCustomElementProp() hasn't been called and which isn't defined by the HTML spec as a global property or attribute, then React can implement some default logic. For example, perhaps:

  • In version 17, maintain BC with v16 and set as an attribute.
  • In version 18, assume that it can be either and follow the most optimal strategy for that.

But in any case, by keeping this a separate API, the JSX and props objects are kept clean and within a single namespace, just like they are for React components and non-custom HTML elements.

effulgentsia commented Oct 24, 2017

It might be the case that on the server the sigil would fallback to setting an attribute.

I don't think that would work if the reason for the sigil is that it's a property that doesn't exist as an attribute, such as video's currentTime.

differentiate using a property because you "can" from using a property because you "must"

I think this differentiation is important, because there's entirely different reasons for choosing to use an attribute or property as an optimization (e.g., SSR preferring attributes vs. client-side rendering preferring properties) vs. something that exists either as only an attribute or only a property.

With regard to server-side rendering, could that problem be solved by providing an API for application code to inform React on how to map custom element properties to attributes?

To be more specific, I'm suggesting something like this:

ReactDOM.defineCustomElementProp(elementName, propName, domPropertyName, htmlAttributeName, attributeSerializer)

Examples:

// 'muted' can be set as either a property or an attribute.
ReactDOM.defineCustomElementProp('x-foo', 'muted', 'muted', 'muted')

// 'currentTime' can only be set as a property.
ReactDOM.defineCustomElementProp('x-foo', 'currentTime', 'currentTime', null)

// 'my-attribute' can only be set as an attribute.
ReactDOM.defineCustomElementProp('x-foo', 'my-attribute', null, 'my-attribute')

// 'richData' can be set as either a property or an attribute.
// When setting as an attribute, set it as a JSON string rather than "[object Object]".
ReactDOM.defineCustomElementProp('x-foo', 'richData', 'richData', 'richdata', JSON.stringify)

For something that can only be a property (where htmlAttributeName is null), SSR would skip over rendering it and then hydrate it on the client.

For something that can only be an attribute (where domPropertyName is null), React would invoke setAttribute() as currently in v16.

For something that can be both, React could choose whatever strategy is most optimal. Perhaps that means always setting as a property on client-side, but as an attribute server-side. Perhaps it means setting as an attribute when initially creating the element, but setting as a property when later patching from the vdom. Perhaps it means only setting as an attribute when the value is a primitive type. Ideally, React should be able to change the strategy whenever it wants to as an internal implementation detail.

When React encounters a prop for which defineCustomElementProp() hasn't been called and which isn't defined by the HTML spec as a global property or attribute, then React can implement some default logic. For example, perhaps:

  • In version 17, maintain BC with v16 and set as an attribute.
  • In version 18, assume that it can be either and follow the most optimal strategy for that.

But in any case, by keeping this a separate API, the JSX and props objects are kept clean and within a single namespace, just like they are for React components and non-custom HTML elements.

@effulgentsia

This comment has been minimized.

Show comment
Hide comment
@effulgentsia

effulgentsia Oct 25, 2017

Sorry for the excessive comments, but I thought of another benefit to my proposal above that I'd like to share:

Those ReactDOM.defineCustomElementProp() calls could be provided in a JS file maintained by the custom element author (in the same repository as where the custom element is maintained/distributed). It wouldn't be needed for custom elements with a strict 1:1 correspondence of property/attribute, which per this issue's Background statement is the recommendation and majority case anyway. So only custom element authors not following this recommendation would need to provide the React integration file. If the author doesn't provide it (e.g., because the custom element author doesn't care about React), then the community of people who use that custom element within React apps could self-organize a central repository for housing that integration file.

I think the possibility of such centralization is preferable to a solution that requires every user of the custom element to always have to be explicit with a sigil.

effulgentsia commented Oct 25, 2017

Sorry for the excessive comments, but I thought of another benefit to my proposal above that I'd like to share:

Those ReactDOM.defineCustomElementProp() calls could be provided in a JS file maintained by the custom element author (in the same repository as where the custom element is maintained/distributed). It wouldn't be needed for custom elements with a strict 1:1 correspondence of property/attribute, which per this issue's Background statement is the recommendation and majority case anyway. So only custom element authors not following this recommendation would need to provide the React integration file. If the author doesn't provide it (e.g., because the custom element author doesn't care about React), then the community of people who use that custom element within React apps could self-organize a central repository for housing that integration file.

I think the possibility of such centralization is preferable to a solution that requires every user of the custom element to always have to be explicit with a sigil.

@LeeCheneler

This comment has been minimized.

Show comment
Hide comment
@LeeCheneler

LeeCheneler Oct 25, 2017

Option 3 would be my preferred but that's a huge breaking change... What about the inverse? Attributes have a prefix not props?

LeeCheneler commented Oct 25, 2017

Option 3 would be my preferred but that's a huge breaking change... What about the inverse? Attributes have a prefix not props?

@drcmda

This comment has been minimized.

Show comment
Hide comment
@drcmda

drcmda Oct 25, 2017

Sigils in React, i don't know how i feel about it. The JSX spec should be regarded as universal, not overly reliant or dependent on browser specifics, especially not on irregularities due to backward compatibility. obj[prop] = value and obj.setAttributes(props, value) behaving different is unfortunate but looking at the browsers api as a whole, not a surprise. @ : [] would make a implementation detail leak to the surface and contradict the javascript centric approach. So unless we have a spec that does the following i think it's a bad idea: const data = @myFunction // -> "[object Object]"

If i have to rely on a web component, i'd be glad if the semantics are hidden away from React and JSX as well as making sure they do not introduce breaking changes. From all the options, leaving ref => ... in place seems to be favourable to me. ref is specifically designed to access the object. And at least the developer knows exactly what's going on, there's no sigil-leak, neither new attributes that could break existing projects.

drcmda commented Oct 25, 2017

Sigils in React, i don't know how i feel about it. The JSX spec should be regarded as universal, not overly reliant or dependent on browser specifics, especially not on irregularities due to backward compatibility. obj[prop] = value and obj.setAttributes(props, value) behaving different is unfortunate but looking at the browsers api as a whole, not a surprise. @ : [] would make a implementation detail leak to the surface and contradict the javascript centric approach. So unless we have a spec that does the following i think it's a bad idea: const data = @myFunction // -> "[object Object]"

If i have to rely on a web component, i'd be glad if the semantics are hidden away from React and JSX as well as making sure they do not introduce breaking changes. From all the options, leaving ref => ... in place seems to be favourable to me. ref is specifically designed to access the object. And at least the developer knows exactly what's going on, there's no sigil-leak, neither new attributes that could break existing projects.

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Oct 25, 2017

@LeeCheneler

Option 3 would be my preferred but that's a huge breaking change... What about the inverse? Attributes have a prefix not props?

Why would it be a breaking change? The current behavior of attributes being the default would remain. The sigil would be opt-in and developers would use it to replace the spots in their code where they currently use a ref to pass data to a custom element as a JS property.

@drcmda

neither new attributes that could break existing projects.

Can you clarify what you meant by this?

robdodson commented Oct 25, 2017

@LeeCheneler

Option 3 would be my preferred but that's a huge breaking change... What about the inverse? Attributes have a prefix not props?

Why would it be a breaking change? The current behavior of attributes being the default would remain. The sigil would be opt-in and developers would use it to replace the spots in their code where they currently use a ref to pass data to a custom element as a JS property.

@drcmda

neither new attributes that could break existing projects.

Can you clarify what you meant by this?

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Oct 25, 2017

FYI for anyone following the discussion, I've updated the RFC with a 5th option suggested by members of the React team.

robdodson commented Oct 25, 2017

FYI for anyone following the discussion, I've updated the RFC with a 5th option suggested by members of the React team.

@drcmda

This comment has been minimized.

Show comment
Hide comment
@drcmda

drcmda Oct 25, 2017

@robdodson

I was referring to this:

Option 4: Add an attributes object
Cons
It may be a breaking change

drcmda commented Oct 25, 2017

@robdodson

I was referring to this:

Option 4: Add an attributes object
Cons
It may be a breaking change

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Oct 25, 2017

Member

Option 5 seems safest for us. It lets us add the feature without having to make a decision about “implicit” API right now since the ecosystem is still in the “figuring it out” phase. We can always revisit it in a few years.

Polymer’s paper-input element has 37 properties, so it would produce a very large config. If developers are using a lot of custom elements in their app, that may equal a lot of configs they need to write.

My impression is that custom element users in React will eventually want to wrap some custom elements into React components anyway for app-specific behavior/customizations. It is a nicer migration strategy for this case if everything already is a React component, e.g.

import XButton from './XButton';

and that happens to be generated by

export default ReactDOM.createCustomElementType(...)

This lets them replace a React component with a custom component that uses (or even doesn’t use) custom elements at any point in time.

So, if people are going to create React components at interop points, we might as well provide a powerful helper to do so. It is also likely that people will share those configs for custom elements they use.

And eventually, if we see the ecosystem stabilize, we can adopt a config-less approach.

I think the next step here would be to write a detailed proposal for how the config should look like to satisfy all common use cases. It should be compelling enough for custom element + React users, since if it doesn't answer common use cases (like event handling) we're going to end up in the limbo where the feature doesn't provide enough benefit to offset the verbosity.

Member

gaearon commented Oct 25, 2017

Option 5 seems safest for us. It lets us add the feature without having to make a decision about “implicit” API right now since the ecosystem is still in the “figuring it out” phase. We can always revisit it in a few years.

Polymer’s paper-input element has 37 properties, so it would produce a very large config. If developers are using a lot of custom elements in their app, that may equal a lot of configs they need to write.

My impression is that custom element users in React will eventually want to wrap some custom elements into React components anyway for app-specific behavior/customizations. It is a nicer migration strategy for this case if everything already is a React component, e.g.

import XButton from './XButton';

and that happens to be generated by

export default ReactDOM.createCustomElementType(...)

This lets them replace a React component with a custom component that uses (or even doesn’t use) custom elements at any point in time.

So, if people are going to create React components at interop points, we might as well provide a powerful helper to do so. It is also likely that people will share those configs for custom elements they use.

And eventually, if we see the ecosystem stabilize, we can adopt a config-less approach.

I think the next step here would be to write a detailed proposal for how the config should look like to satisfy all common use cases. It should be compelling enough for custom element + React users, since if it doesn't answer common use cases (like event handling) we're going to end up in the limbo where the feature doesn't provide enough benefit to offset the verbosity.

@effulgentsia

This comment has been minimized.

Show comment
Hide comment
@effulgentsia

effulgentsia Oct 25, 2017

Building from my earlier comment, how about:

const XFoo = ReactDOM.createCustomElementType('x-foo', {
  propName1: {
    propertyName: string | null,
    attributeName: string | null,
    attributeSerializer: function | null,
    eventName: string | null,
  }
  propName2: {
  }
  ...
});

The logic would then be, for each React prop on an XFoo instance:

  1. If the eventName for that prop is not null, register it as an event handler that invokes the prop value (assumed to be a function).
  2. Else if rendering client-side and propertyName is not null, set the element property to the prop value.
  3. Else if attributeName is not null, set the element attribute to the stringified prop value. If attributeSerializer is not null, use it to stringify the prop value. Otherwise, just do '' + propValue.

Polymer’s paper-input element has 37 properties, so it would produce a very large config.

I'd like to suggest that the config only be necessary for outlier props. For any prop on the XFoo instance that wasn't included in the config, default it to:

  • if the value is a function:
eventName: the prop name,
  • else:
propertyName: the prop name,
attributeName: camelCaseToDashCase(the prop name),

effulgentsia commented Oct 25, 2017

Building from my earlier comment, how about:

const XFoo = ReactDOM.createCustomElementType('x-foo', {
  propName1: {
    propertyName: string | null,
    attributeName: string | null,
    attributeSerializer: function | null,
    eventName: string | null,
  }
  propName2: {
  }
  ...
});

The logic would then be, for each React prop on an XFoo instance:

  1. If the eventName for that prop is not null, register it as an event handler that invokes the prop value (assumed to be a function).
  2. Else if rendering client-side and propertyName is not null, set the element property to the prop value.
  3. Else if attributeName is not null, set the element attribute to the stringified prop value. If attributeSerializer is not null, use it to stringify the prop value. Otherwise, just do '' + propValue.

Polymer’s paper-input element has 37 properties, so it would produce a very large config.

I'd like to suggest that the config only be necessary for outlier props. For any prop on the XFoo instance that wasn't included in the config, default it to:

  • if the value is a function:
eventName: the prop name,
  • else:
propertyName: the prop name,
attributeName: camelCaseToDashCase(the prop name),
@effulgentsia

This comment has been minimized.

Show comment
Hide comment
@effulgentsia

effulgentsia Oct 25, 2017

Alternatively, maybe it makes sense to keep events in a separate namespace, in which case, remove everything having to do with eventName from the last comment, and instead let events be registered as:

<XFoo prop1={propValue1} prop2={propValue2} events={event1: functionFoo, event2: functionBar}>
</XFoo>

effulgentsia commented Oct 25, 2017

Alternatively, maybe it makes sense to keep events in a separate namespace, in which case, remove everything having to do with eventName from the last comment, and instead let events be registered as:

<XFoo prop1={propValue1} prop2={propValue2} events={event1: functionFoo, event2: functionBar}>
</XFoo>
@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Oct 25, 2017

@gaearon @effulgentsia what do y'all think of a combination of option 1 and option 5?

Option 1 would make it easier for the casual user of a custom element to pass rich data. I'm imagining the scenario where I'm building an app and I just want to use a couple of custom elements. I already know how they work and I'm not so invested that I want to write a config for them.

Option 5 would be for folks who want to use something like paper-input all over their app and would really like to expose its entire API to everyone on their team.

robdodson commented Oct 25, 2017

@gaearon @effulgentsia what do y'all think of a combination of option 1 and option 5?

Option 1 would make it easier for the casual user of a custom element to pass rich data. I'm imagining the scenario where I'm building an app and I just want to use a couple of custom elements. I already know how they work and I'm not so invested that I want to write a config for them.

Option 5 would be for folks who want to use something like paper-input all over their app and would really like to expose its entire API to everyone on their team.

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Oct 25, 2017

For SSR of option 1 the heuristic could be always use an attribute if rendering on the server. A camelCase property gets converted to a dash-case attribute. That seems to be a pretty common pattern across web component libraries.

robdodson commented Oct 25, 2017

For SSR of option 1 the heuristic could be always use an attribute if rendering on the server. A camelCase property gets converted to a dash-case attribute. That seems to be a pretty common pattern across web component libraries.

@effulgentsia

This comment has been minimized.

Show comment
Hide comment
@effulgentsia

effulgentsia Oct 25, 2017

I like the idea of an option1 + option5 combination a lot. Meaning that for most custom elements:

<x-foo prop1={propValue1}>

would work as expected: prop1 set as a property client-side and as a (dash-cased) attribute server-side.

And people could switch to option5 for anything for which the above doesn't suit them.

It would be a breaking change though from the way React 16 works. For anyone who experiences that breakage (e.g., they were using a custom element with attributes that aren't backed by properties), they could switch to option5, but it's still a break. I leave it to the React team to decide if that's acceptable.

effulgentsia commented Oct 25, 2017

I like the idea of an option1 + option5 combination a lot. Meaning that for most custom elements:

<x-foo prop1={propValue1}>

would work as expected: prop1 set as a property client-side and as a (dash-cased) attribute server-side.

And people could switch to option5 for anything for which the above doesn't suit them.

It would be a breaking change though from the way React 16 works. For anyone who experiences that breakage (e.g., they were using a custom element with attributes that aren't backed by properties), they could switch to option5, but it's still a break. I leave it to the React team to decide if that's acceptable.

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Nov 3, 2017

I really like this approach :)

robdodson commented Nov 3, 2017

I really like this approach :)

thysultan referenced this issue in thysultan/dio.js Nov 14, 2017

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Dec 12, 2017

Friendly ping on this. @gaearon @sophiebits do y'all have any thoughts on @effulgentsia's latest proposal? Just curious if it's in the ballpark or a non-starter.

robdodson commented Dec 12, 2017

Friendly ping on this. @gaearon @sophiebits do y'all have any thoughts on @effulgentsia's latest proposal? Just curious if it's in the ballpark or a non-starter.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Dec 12, 2017

Member

We just opened an RFC process. Could we ask either of you to submit it as an RFC?
https://github.com/reactjs/rfcs

Member

gaearon commented Dec 12, 2017

We just opened an RFC process. Could we ask either of you to submit it as an RFC?
https://github.com/reactjs/rfcs

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Dec 12, 2017

Member

I would rather not add domProperties and eventListeners "props" (or the equivalent within a ref={{}} object) because it makes using custom elements very unnatural and unlike all other React components. If the component user is going to need to acutely know the difference between properties and attributes, etc, then I would rather do it as a ReactDOM.createCustomElementType-style solution. Then if you use the component exactly once it is a comparable amount of work (specifying the configuration and then using it once), but if you use the component many times then you don't need to think about the configuration object every time. Requiring that the configuration be specified every time seems to defeat the goals of having a clean custom elements integration unless I'm missing something.

Member

sophiebits commented Dec 12, 2017

I would rather not add domProperties and eventListeners "props" (or the equivalent within a ref={{}} object) because it makes using custom elements very unnatural and unlike all other React components. If the component user is going to need to acutely know the difference between properties and attributes, etc, then I would rather do it as a ReactDOM.createCustomElementType-style solution. Then if you use the component exactly once it is a comparable amount of work (specifying the configuration and then using it once), but if you use the component many times then you don't need to think about the configuration object every time. Requiring that the configuration be specified every time seems to defeat the goals of having a clean custom elements integration unless I'm missing something.

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Dec 12, 2017

@sophiebits OK, I could attempt to write up an RFC for something like that. What do you think about the idea you floated back on October 26 of going properties first on the client side and also allowing folks to write ReactDOM.createCustomElementType for SSR and/or if they want really fine grained control over how the client maps attrs/properties?

robdodson commented Dec 12, 2017

@sophiebits OK, I could attempt to write up an RFC for something like that. What do you think about the idea you floated back on October 26 of going properties first on the client side and also allowing folks to write ReactDOM.createCustomElementType for SSR and/or if they want really fine grained control over how the client maps attrs/properties?

@treshugart

This comment has been minimized.

Show comment
Hide comment
@treshugart

treshugart Dec 12, 2017

At least with the createCustomElementType style, libraries can easily map their APIs into that. I.e., our skatejs/renderer-react could easily take the props and configure the custom element for React. This sort of leaves vanilla folks high and dry without an abstraction or performing a bit of work, though. I like Rob's suggestion of a safe default, while allowing fine-grained control. Is that something that would work?

treshugart commented Dec 12, 2017

At least with the createCustomElementType style, libraries can easily map their APIs into that. I.e., our skatejs/renderer-react could easily take the props and configure the custom element for React. This sort of leaves vanilla folks high and dry without an abstraction or performing a bit of work, though. I like Rob's suggestion of a safe default, while allowing fine-grained control. Is that something that would work?

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Dec 13, 2017

Member

@robdodson I think I'm on board with it. Can't promise no other concerns will come up but I think it feels like a good balance.

Member

sophiebits commented Dec 13, 2017

@robdodson I think I'm on board with it. Can't promise no other concerns will come up but I think it feels like a good balance.

@trusktr

This comment has been minimized.

Show comment
Hide comment
@trusktr

trusktr Jan 5, 2018

Option 3 is the best option so far, and it can work with SSR, I'll explain an idea of how.

So far all options by themselves, except for 3,

  • either create high maintenance burden for everyone outside of React source
  • or they force all custom element authors in the entire universe to abide by React-specific rules.

Here's universal rules we all agree on:

  • setting attributes with setAttribute is the most standard way in the universe for passing data to elements in a way that matches 1-to-1 with Declarative HTML attributes. This has to work 100% of the time as a law of the universe, therefore it's the only 100% guaranteed way to pass data to elements.
  • some people aren't happy that it was designed only for strings.
  • Some elements (I repeat, only some elements), accept values via object properties that map to certain attributes. This is not something that can be relied on 100% of the time.
  • some people like object properties because they can accept non-strings

So, at bare minimum, if React wants to work 100% with every single custom element in the universe and not be a burden for people, then:

  • React needs to realize it isn't God, there's other many other libraries that people use besides react,
  • therefore React shoud by default just pass data via setAttribute because that is 100% standard.
  • React should accept the fact that Custom Element authors can extend/override the setAttribute methods in their class definitions, making setAttribute accept things other than strings. A prime example can be libraries like A-frame.
  • React should accept that if a custom element author wants a custom element to work with every possible library, not just with React then that author will rely on setAttribute to make his element by default compatible with everything, and if by default all libraries rely on attributes, then the whole universe will work with each other. There's no ifs, ands, or buts about this! (unless the w3c/whatwg makes some big changes)

Soooooooo, that being said, we should at the very least

  • fix #10070 (where I was redirected from)
  • realize that doing this gets us simply closer to standards

Then, we can think about the implications of elements sometimes having object-property API:

  • developers have the choice to use setAttribute knowing it will work all the time.
  • developers can sometimes take advantage of object properties.
  • developers always have to be aware whether or not an object-property interface exists for any given element (custom or not).
  • object-property interfaces are an alternative interface that don't necessarily map 1-to-1 with attributes.

So, with this knowledge of attributes vs properties, a solution in React that wishes to augment the 100% standard and respect laws of the universe should:

  1. allow attributes to work 100% of the time by default. This means that <x-foo blah="blah" /> should by default map to setAttribute and pass the value along unchanged. This is a non-breaking change. In fact, it is a fixing change that would otherwise result in meaningless "[object Object]" string being passed in.
  2. Come up with an alternative way to let the React user optionally use props if the user is conscious about which object-property interfaces exist and wants explicitly use those interfaces.

It seems like Option 3, using a sigil (who's extra syntax is honestly not hard to learn), is a solution that gets closest to ideal. Based on this SO question, then the only available symbol is =, though settling on something like & (with an escapable form, perhaps like \&) is more readable. For example, if I want a prop specifically:

<x-foo &blah="blah" />

Most other characters covered by the WHATWG HTML syntax spec should work, and I hope that they do but that's another topic.

Option 3 is the best option so far. How can SSR work? Just serialize the prop data (with known limitations), then set the object properties on the client side during hydration.

If hydration isn't being used on the client and therefore props don't make sense in SSR, then, oh well. It never worked before, and it doesn't need to work now. PHP-style or Java-style SSR sends static HTML with no hydration and they rely 100% on attributes. It goes to say, if we use React SSR, we will probably use client-side hydration, and if we don't want hydration, then we should simply be aware of the fact that we shouldn't use props in this case. This is simple. This is how the web works. All that react has to do is make this caveat clear in the documentation.

But!!! That's not all! We can still include the features of Option 5 to give people more control. With an API like Option 5,

  • Some people can configure how some attributes can map to props
  • AND how some props can map to attribues. This can help people make SSR work even for the non-hydrating type of SSR!
  • we can let people define "if this attribute is written, actually pass it to this prop, but if it is SSR, don't pass it to the prop", or "pass this prop to this attribute only during SSR, otherwise do what I specified using the sigil", etc

In the end, the following seems like a solution that would work:

  • Option 3 with setAttribute used by default behind the scenes,
  • with a fix for #10070 so that React doesn't get in people's way,
  • and an API like in Option 5 for further tuning for those that really need it, including ways to tune SSR, client, or both.

Happy new year!

trusktr commented Jan 5, 2018

Option 3 is the best option so far, and it can work with SSR, I'll explain an idea of how.

So far all options by themselves, except for 3,

  • either create high maintenance burden for everyone outside of React source
  • or they force all custom element authors in the entire universe to abide by React-specific rules.

Here's universal rules we all agree on:

  • setting attributes with setAttribute is the most standard way in the universe for passing data to elements in a way that matches 1-to-1 with Declarative HTML attributes. This has to work 100% of the time as a law of the universe, therefore it's the only 100% guaranteed way to pass data to elements.
  • some people aren't happy that it was designed only for strings.
  • Some elements (I repeat, only some elements), accept values via object properties that map to certain attributes. This is not something that can be relied on 100% of the time.
  • some people like object properties because they can accept non-strings

So, at bare minimum, if React wants to work 100% with every single custom element in the universe and not be a burden for people, then:

  • React needs to realize it isn't God, there's other many other libraries that people use besides react,
  • therefore React shoud by default just pass data via setAttribute because that is 100% standard.
  • React should accept the fact that Custom Element authors can extend/override the setAttribute methods in their class definitions, making setAttribute accept things other than strings. A prime example can be libraries like A-frame.
  • React should accept that if a custom element author wants a custom element to work with every possible library, not just with React then that author will rely on setAttribute to make his element by default compatible with everything, and if by default all libraries rely on attributes, then the whole universe will work with each other. There's no ifs, ands, or buts about this! (unless the w3c/whatwg makes some big changes)

Soooooooo, that being said, we should at the very least

  • fix #10070 (where I was redirected from)
  • realize that doing this gets us simply closer to standards

Then, we can think about the implications of elements sometimes having object-property API:

  • developers have the choice to use setAttribute knowing it will work all the time.
  • developers can sometimes take advantage of object properties.
  • developers always have to be aware whether or not an object-property interface exists for any given element (custom or not).
  • object-property interfaces are an alternative interface that don't necessarily map 1-to-1 with attributes.

So, with this knowledge of attributes vs properties, a solution in React that wishes to augment the 100% standard and respect laws of the universe should:

  1. allow attributes to work 100% of the time by default. This means that <x-foo blah="blah" /> should by default map to setAttribute and pass the value along unchanged. This is a non-breaking change. In fact, it is a fixing change that would otherwise result in meaningless "[object Object]" string being passed in.
  2. Come up with an alternative way to let the React user optionally use props if the user is conscious about which object-property interfaces exist and wants explicitly use those interfaces.

It seems like Option 3, using a sigil (who's extra syntax is honestly not hard to learn), is a solution that gets closest to ideal. Based on this SO question, then the only available symbol is =, though settling on something like & (with an escapable form, perhaps like \&) is more readable. For example, if I want a prop specifically:

<x-foo &blah="blah" />

Most other characters covered by the WHATWG HTML syntax spec should work, and I hope that they do but that's another topic.

Option 3 is the best option so far. How can SSR work? Just serialize the prop data (with known limitations), then set the object properties on the client side during hydration.

If hydration isn't being used on the client and therefore props don't make sense in SSR, then, oh well. It never worked before, and it doesn't need to work now. PHP-style or Java-style SSR sends static HTML with no hydration and they rely 100% on attributes. It goes to say, if we use React SSR, we will probably use client-side hydration, and if we don't want hydration, then we should simply be aware of the fact that we shouldn't use props in this case. This is simple. This is how the web works. All that react has to do is make this caveat clear in the documentation.

But!!! That's not all! We can still include the features of Option 5 to give people more control. With an API like Option 5,

  • Some people can configure how some attributes can map to props
  • AND how some props can map to attribues. This can help people make SSR work even for the non-hydrating type of SSR!
  • we can let people define "if this attribute is written, actually pass it to this prop, but if it is SSR, don't pass it to the prop", or "pass this prop to this attribute only during SSR, otherwise do what I specified using the sigil", etc

In the end, the following seems like a solution that would work:

  • Option 3 with setAttribute used by default behind the scenes,
  • with a fix for #10070 so that React doesn't get in people's way,
  • and an API like in Option 5 for further tuning for those that really need it, including ways to tune SSR, client, or both.

Happy new year!

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Jan 22, 2018

I want to respond to some of the above points, but fear that this thread is already incredibly long. So, sorry for making it longer :P

Here's universal rules we all agree on:

setting attributes with setAttribute is the most standard way in the universe for passing data to elements in a way that matches 1-to-1 with Declarative HTML attributes. This has to work 100% of the time as a law of the universe, therefore it's the only 100% guaranteed way to pass data to elements.

This isn't entirely true. There is nothing in the platform to enforce that a custom element expose an attributes interface. You could just as easily create one that only accepts JS properties. So it's not a "guaranteed way to pass data". The lack of enforcement mechanisms means that you can't rely on either style (HTML attributes or JS properties) with 100% certainty.

some people aren't happy that it was designed only for strings.
Some elements (I repeat, only some elements), accept values via object properties that map to certain attributes. This is not something that can be relied on 100% of the time.

We encourage folks to not even bother creating attributes for properties which accept rich data like objects/arrays. This is because some data cannot be serialized back to an attribute string. For example, if one of your object properties is a reference to a DOM Node, that can't actually be stringified. Also, when you stringify and reparse an object, it loses its identity. Meaning, if it has a reference to another POJO, you can't actually use that reference since you've created an entirely new object.

some people like object properties because they can accept non-strings

therefore React shoud by default just pass data via setAttribute because that is 100% standard.

JavaScript properties are equally as standard. Most HTML elements expose both an attributes and corresponding properties interface. For example, <img src=""> or HTMLImageElement.src.

React should accept the fact that Custom Element authors can extend/override the setAttribute methods in their class definitions, making setAttribute accept things other than strings.

Authors could do that, but that actually seems way more "non-standard" than just using JS properties. It may also expose you to weird issues related to parsing and cloning the element.

React should accept that if a custom element author wants a custom element to work with every possible library, not just with React then that author will rely on setAttribute to make his element by default compatible with everything, and if by default all libraries rely on attributes, then the whole universe will work with each other. There's no ifs, ands, or buts about this! (unless the w3c/whatwg makes some big changes)

I'm not sure how you arrive at this conclusion because there are other libraries which prefer setting JS properties on custom elements (Angular, for example). For maximum compatibility, authors should back their attributes with JS properties. That will cover the largest surface area of possible uses. All elements created with Polymer do this by default.

allow attributes to work 100% of the time by default. This means that should by default map to setAttribute and pass the value along unchanged. This is a non-breaking change. In fact, it is a fixing change that would otherwise result in meaningless "[object Object]" string being passed in.

I think React actually is passing the value unchanged. Calling setAttribute('foo', {some: object}) results in [object Object]. Unless you're proposing that they call JSON.stringify() on the object? But then that object isn't "unchanged." I think maybe you're relying on the author to have overridden setAttribute()? It may be more plausible to encourage them to create corresponding JS properties instead of monkey-patching the DOM.

robdodson commented Jan 22, 2018

I want to respond to some of the above points, but fear that this thread is already incredibly long. So, sorry for making it longer :P

Here's universal rules we all agree on:

setting attributes with setAttribute is the most standard way in the universe for passing data to elements in a way that matches 1-to-1 with Declarative HTML attributes. This has to work 100% of the time as a law of the universe, therefore it's the only 100% guaranteed way to pass data to elements.

This isn't entirely true. There is nothing in the platform to enforce that a custom element expose an attributes interface. You could just as easily create one that only accepts JS properties. So it's not a "guaranteed way to pass data". The lack of enforcement mechanisms means that you can't rely on either style (HTML attributes or JS properties) with 100% certainty.

some people aren't happy that it was designed only for strings.
Some elements (I repeat, only some elements), accept values via object properties that map to certain attributes. This is not something that can be relied on 100% of the time.

We encourage folks to not even bother creating attributes for properties which accept rich data like objects/arrays. This is because some data cannot be serialized back to an attribute string. For example, if one of your object properties is a reference to a DOM Node, that can't actually be stringified. Also, when you stringify and reparse an object, it loses its identity. Meaning, if it has a reference to another POJO, you can't actually use that reference since you've created an entirely new object.

some people like object properties because they can accept non-strings

therefore React shoud by default just pass data via setAttribute because that is 100% standard.

JavaScript properties are equally as standard. Most HTML elements expose both an attributes and corresponding properties interface. For example, <img src=""> or HTMLImageElement.src.

React should accept the fact that Custom Element authors can extend/override the setAttribute methods in their class definitions, making setAttribute accept things other than strings.

Authors could do that, but that actually seems way more "non-standard" than just using JS properties. It may also expose you to weird issues related to parsing and cloning the element.

React should accept that if a custom element author wants a custom element to work with every possible library, not just with React then that author will rely on setAttribute to make his element by default compatible with everything, and if by default all libraries rely on attributes, then the whole universe will work with each other. There's no ifs, ands, or buts about this! (unless the w3c/whatwg makes some big changes)

I'm not sure how you arrive at this conclusion because there are other libraries which prefer setting JS properties on custom elements (Angular, for example). For maximum compatibility, authors should back their attributes with JS properties. That will cover the largest surface area of possible uses. All elements created with Polymer do this by default.

allow attributes to work 100% of the time by default. This means that should by default map to setAttribute and pass the value along unchanged. This is a non-breaking change. In fact, it is a fixing change that would otherwise result in meaningless "[object Object]" string being passed in.

I think React actually is passing the value unchanged. Calling setAttribute('foo', {some: object}) results in [object Object]. Unless you're proposing that they call JSON.stringify() on the object? But then that object isn't "unchanged." I think maybe you're relying on the author to have overridden setAttribute()? It may be more plausible to encourage them to create corresponding JS properties instead of monkey-patching the DOM.

@trusktr

This comment has been minimized.

Show comment
Hide comment
@trusktr

trusktr Jan 23, 2018

I think React actually is passing the value unchanged. Calling setAttribute('foo', {some: object}) results in [object Object]

React is coercing values to a string before passing into setAttribute:

node.setAttribute(attributeName, '' + (value: any));

and

attributeValue = '' + (value: any);

I basically agree with all you said.

We agree that people are doing things both ways, and there's not a standard that forces everyone to do it just one way or the other, so I still think

  • Option 3 with setAttribute used by default and a sigil for specifying to use instance properties,
  • with a fix for #10070 so that React doesn't coerce args to strings,
  • and Option 5, an API for fine tuning

If Option 5 is done well, then hydration for SSR solutions can map data to either attributes or props as can be specified by the use of Option 5 API.

trusktr commented Jan 23, 2018

I think React actually is passing the value unchanged. Calling setAttribute('foo', {some: object}) results in [object Object]

React is coercing values to a string before passing into setAttribute:

node.setAttribute(attributeName, '' + (value: any));

and

attributeValue = '' + (value: any);

I basically agree with all you said.

We agree that people are doing things both ways, and there's not a standard that forces everyone to do it just one way or the other, so I still think

  • Option 3 with setAttribute used by default and a sigil for specifying to use instance properties,
  • with a fix for #10070 so that React doesn't coerce args to strings,
  • and Option 5, an API for fine tuning

If Option 5 is done well, then hydration for SSR solutions can map data to either attributes or props as can be specified by the use of Option 5 API.

@robdodson

This comment has been minimized.

Show comment
Hide comment
@robdodson

robdodson Jan 23, 2018

React is coercing values to a string before passing into setAttribute

I see. Since most folks don't define a custom toString() it defaults to [object Object].

robdodson commented Jan 23, 2018

React is coercing values to a string before passing into setAttribute

I see. Since most folks don't define a custom toString() it defaults to [object Object].

@trusktr

This comment has been minimized.

Show comment
Hide comment
@trusktr

trusktr Jan 23, 2018

Since most folks don't define a custom toString() it defaults to [object Object].

Just like if I do

const div = document.createElement('div')
div.setAttribute('foo', {a:1, b:2, c:3})

the result is

<div foo="[object Object]"></div>

Obviously as a web developers we should be aware what happens when we pass a non-strings into element attributes. For example I'm aware that I can pass non-strings into A-Frame elements, and I should be free to do that without a library getting in the way.

trusktr commented Jan 23, 2018

Since most folks don't define a custom toString() it defaults to [object Object].

Just like if I do

const div = document.createElement('div')
div.setAttribute('foo', {a:1, b:2, c:3})

the result is

<div foo="[object Object]"></div>

Obviously as a web developers we should be aware what happens when we pass a non-strings into element attributes. For example I'm aware that I can pass non-strings into A-Frame elements, and I should be free to do that without a library getting in the way.

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Jan 23, 2018

Member

React needs to realize it isn't God, there's other many other libraries that people use besides react

This is unnecessarily snarky. You'll note in this thread that we do care about this but that there are many different options and visions for where to take the custom element design. It's certainly not obvious what should be done.

Member

sebmarkbage commented Jan 23, 2018

React needs to realize it isn't God, there's other many other libraries that people use besides react

This is unnecessarily snarky. You'll note in this thread that we do care about this but that there are many different options and visions for where to take the custom element design. It's certainly not obvious what should be done.

@trusktr

This comment has been minimized.

Show comment
Hide comment
@trusktr

trusktr Jan 23, 2018

@sebmarkbage Sorry about that, I didn't mean to be snarky at all, and I think React is a nice lib. I should've thought about my words more carefully there (especially because not everyone has the same religion).

What I meant is, React is very popular, so React has the potential to sway people to do things in a certain way that may not work in other places (f.e. it could tell people to rely on instance properties for all Custom Elements which wouldn't work with all Custom Elements).

React currently converts all values passed to element attributes into strings. Had React not done this, for example, there would be no need for aframe-react (which works around the string problem) to even exist.

If React can just let us have the ability to make any choice about how we pass data to elements, just like in plain JavaScript, that'd make me the most satisfied user. 😊

Again, sorry about my choice of words there. I'll double think it next time.

trusktr commented Jan 23, 2018

@sebmarkbage Sorry about that, I didn't mean to be snarky at all, and I think React is a nice lib. I should've thought about my words more carefully there (especially because not everyone has the same religion).

What I meant is, React is very popular, so React has the potential to sway people to do things in a certain way that may not work in other places (f.e. it could tell people to rely on instance properties for all Custom Elements which wouldn't work with all Custom Elements).

React currently converts all values passed to element attributes into strings. Had React not done this, for example, there would be no need for aframe-react (which works around the string problem) to even exist.

If React can just let us have the ability to make any choice about how we pass data to elements, just like in plain JavaScript, that'd make me the most satisfied user. 😊

Again, sorry about my choice of words there. I'll double think it next time.

@treshugart

This comment has been minimized.

Show comment
Hide comment
@treshugart

treshugart Feb 2, 2018

I've added a comment to the RFC PR for this. I think it's worth discussing as it covers what's being proposed as well as a simpler model for reliably inferring a custom element and its properties. It turns it back into a hybrid approach, but offers a zero-config way of integrating for most use cases.

treshugart commented Feb 2, 2018

I've added a comment to the RFC PR for this. I think it's worth discussing as it covers what's being proposed as well as a simpler model for reliably inferring a custom element and its properties. It turns it back into a hybrid approach, but offers a zero-config way of integrating for most use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment