[WIP] Custom elements example

[shawnbot]

Okay, I'm abandoning #11 and #12 in favor of this PR to demonstrate how we should (I think) go about interacting/integrating with custom elements. Here's the deal:

I spoke with @muan today, and we discussed the merits of a couple of different approaches for integrating github's custom elements with React. Turns out, there are a lot of nuances to each of these, but at a high level we can:

1. Just output custom elements in React, e.g.

   const CustomElement = ({children, ...rest}) => (
     <custom-element {...rest}>{children}</custom-element>
   )

   One thing I really like about this approach is that if we use React to do server-side rendering (e.g. if we get to the point of being able to at least render static HTML from React components on github.com), we can still use the custom element behaviors without the React client-side runtime. (This only really makes sense for "pure"/stateless components.)

   The next decision to make is whether and how the client-side custom element definition gets bundled with the React component. Here, we have a couple more options:

   1. Import it directly so that the bundle always includes the definition:

      import 'custom-element-package'
      // do our thing

   2. Assume — or require? — that the corresponding custom element JS bundle be included elsewhere, e.g. via <script> tag.

   The former is the approach I've taken in this PR by adding details-dialog-element as a versioned dependency. However, if we're going to pursue any server-side-rendered (only) React implementations, we'll probably want to provide a way to expose the custom element definitions client-side as <script> tags.

2. Re-implement custom element behaviors in React components. Mu-An and I agreed that this was generally not a good idea because we'd be duplicating work, and implementations could easily fall out of sync.

3. Create custom element implementations of React components, e.g. with ReactiveElements. We also agreed that this was a non-starter, since it would require the React runtime.

From what I've seen, most of the behaviors that our custom elements implement are actually kind of tricky to pull off in the uni-directional data flow world of React. For instance, the details toggle is difficult to implement because you have to modify the props of the entire component's subtree depending on whether any element declares itself as visible or hidden when the component is "open". For this reason alone I think it makes a lot of sense to entrust most, if not all of the more complicated client-side behaviors to custom elements — at least, for now.

Good

Generally speaking, I rather like the separation of concerns in this PR: React renders the initial DOM state, and the custom element handles the (web) component's state from there on out.

Bad

One downside of this approach is that if we're using React client-side (or hot module reloading), it's entirely possible for custom elements to manipulate the DOM (typically by changing attributes) in such a way that would be overwritten by a subsequent React render pass. For instance:

import receptor from 'receptor'

// pressing escape toggles the hidden attribute
const toggle = receptor.keymap({
  'Escape': function(e) {
    this.hidden = !this.hidden
  }
})

window.customElements.define('escape-toggle', class FooBar extends HTMLElement {
  createdCallback() {
    window.addEventListener('keyup', this.toggle = toggle.bind(this))
  }
})

export default ({hidden=false, children, ...rest}) => (
  <escape-toggle hidden={hidden} {...rest}>
    {children}
  </escape-toggle>
)

In this example, both React and the custom element can modify each <escape-toggle> element's hidden attribute, which might cause issues if a render occurs between keypresses.

A slightly more complicated way of doing things would be to further decompose our behaviors into collections of "pure" (stateless) event listeners that could theoretically work in either browser DOM or React environments. That was kind of the idea behind receptor.behavior, but would need to be re-implemented to work with React specifically.

Further thoughts

I think there's also room for a hybrid approach, which I've hinted at in this details-dialog-element PR. What I'm suggesting there is that custom element packages might export (at the JS module level) constants or other helpers to simplify implementing those behaviors in React, rather than having to remember all of the js-* class names and data-* attributes that make them work. We could even do some work on our end to enforce "validity" of the component's class hierarchy at render time.

Anyway... what do y'all think? 😬 /cc @dgraham @josh

[shawnbot]

This is necessary to support babel's compilation of the custom element class definition because it scopes the HTMLElement identifier in such a way that causes JS errors. I added this, and it worked!

[shawnbot]

Starting to think that all of our projects should have these two config rules...

[shawnbot]

See: https://github.com/github/devtools/pull/169#issuecomment-378760348

[shawnbot]

⚠️ I wasn't sure what the "right" way was to import details-dialog-element's CSS, so I just copy-pasta'd it here.

[shawnbot]

The way that the component is implemented, it's possible for the title to be more complex, e.g.

<DetailsDialog title={<a href="#foo">whatever</a>}>...</DetailsDialog>

[shawnbot]

The data-close-dialog attribute is what triggers a close when clicked.

[muan]

The .Overlay classes & div's shouldn't be needed here with details-dialog's CSS. I think just having Box Box--overlay directly on details-dialog should be enough. Is it not the case?

For reference:

• https://github.com/github/github/blob/95d72037334bfc8b28c71ee566debc29b4f487c5/app/views/settings/user/admin.html.erb#L132-L177
• https://github.com/github/github/blob/95d72037334bfc8b28c71ee566debc29b4f487c5/app/views/orgs/invitations/_invite_member_facebox.html.erb#L1-L41

[shawnbot]

Derp, yeah, I just copied the HTML from the style guide.

[shawnbot]

I looked at this more closely, and there aren't any styles in details-dialog, .Box or .Box--overlay that provide the semi-transparent, dark backdrop. What I was looking to emulate here was the box overlay partial, which wraps .Box.Box--overlay in .Overlay > .Overlay-table-full.bg-transparent-dark > .Overlay-cell-middle. Unless I'm missing something?

[muan]

The dark backdrop from .Overlay doesn't work here because it covers up the full screen, taking away the expanded summary click area which allows no-JS toggle.

Instead, we have .details-expanded.details-expanded-dark in dotcom https://github.com/github/github/blob/dd19df353e080f27b8a0be2964747bd6a73e7861/app/assets/stylesheets/components/details-element.scss.

It is where it is because .details-expanded is used on elements that don't uses details-dialog. That said, they should probably just both live in Primer as utility classes for details!

[josh]

This is a pretty interesting PR! Excited to see where things go.

One thing I really like about this approach is that if we use React to do server-side rendering, we can still use the custom element behaviors without the React client-side runtime.

For sure, the lightweight runtime is a cool fallout. I think there's a potential world were an SSR render takes a second pass at the React element tree and expands Custom Elements that opt into an SSR model.

Import it directly so that the bundle always includes the definition:

I think having the explicit dependency tree is the right direction. It gives the compiler the right information for ensuring those WC dependencies at met at runtime.

While it doesn't work today, something like this is being considered for React 17 where Custom Element constructors could be passed to React.createElement.

import CustomElement from 'custom-element-package'

const StyledCustomElement = styled(
  <CustomElement /> // React.createElement(CustomElement)
)

Create custom element implementations of React components, e.g. with ReactiveElements. We also agreed that this was a non-starter, since it would require the React runtime.

Custom Elements backed by Preact could be an alternative there.

From what I've seen, most of the behaviors that our custom elements implement are actually kind of tricky to pull off in the uni-directional data flow world of React.

Well behaved custom elements should be respectful of uni-directional data flow by responding to props/attr changes. Though a challenge here is that React.createElement is going to serialize it's passed props as string attributes making it difficult to pass structured data. It be nice if you could prefer property setters when available to skip that string serialization step.

One downside of this approach is that if we're using React client-side, it's entirely possible for custom elements to manipulate the DOM in such a way that would be overwritten by a subsequent React render pass.

In this specific example given, I think it's the same problem when a components state can both be initialized via props and then manages its own internal state. Usually this is addressed by naming things differently: props.defaultValue vs state.value which may diverge. This is how React wraps builtins like input by giving the user the ability to treat it as "controlled" where the state always reflects it's props or "uncontrolled" where is state is initialized by props and then the component manages its own state. It'd be interesting to see if theres a reusable pattern for creating wrappers that conform to these ways to using components. Both are useful depending on the context.

[shawnbot]

Thanks for the input, @josh! One common approach I've seen to the "control" (ownership?) issue between React and custom elements is to use class-based components (React.Component) that always return false from shouldComponentUpdate(), which essentially tells React not to update anything in the component's tree after the first render, allowing the custom element to do its magic. E.g., a custom element that managed its ancestors' hidden attributes could expose a props-friendly function that would allow React to "pre-render" the state of its children to avoid the flash of un-hidden content before the element is upgraded client-side:

import {Component} from 'react'
import {deepMap} from 'react-children-utilities'
import {STATE_ATTR, serializeStates, isHiddenProps} from 'hidden-thing-element'

export default class HiddenThing extends Component {
  shouldComponentUpdate() { return false }

  render() {
    const {states=new Set(), children, ...props} = this.props
    props[STATE_ATTR] = serializeStates(states)
    return (
      <hidden-thing {...props}>
        {deepMap(children, child => {
          // use the underlying show/hide logic during the initial render
          child.props.hidden = isHiddenProps(child.props, states)
          return child
        })}
      </hidden-thing>
    )
  }
}

Also, Preact looks really interesting, and might be a nice, lightweight alternative to shipping the React runtime client-side. ✨

[shawnbot]

This is OBE. I'm going to close it and open a new PR for the custom element experiments.