Proposal: Standardize API for methods that return DOM elements

A common pattern that we see today throughout the code of Solid UI is functions that add properties and methods to DOM elements that they then return as result of the function. The element is then treated as normal, but with "magic" properties, of which refresh is probably the most common one.

Having talked with Tim I understand his reasons for exposing methods like refresh, but I would like to propose some alternatives to how we can change the API to avoid adding magic properties to DOM elements.

First of all, let me note a couple of reasons for why we should we avoid the pattern of mutating DOM elements in this way:

• It's not futureproof: If refresh becomes a standard method/property of DOM elements at some point, we have to make sure that all calls on element.refresh are actually done with something like Reflect.apply when we want to the original standard refresh, and not accidently trigger our special addition.
• It's difficult to discover: Since it's adding properties to DOM elements, a practice that is shunned by many front-end developers, many will never discover it unless they read about it somewhere.

The simple way

We could simply change the API of the Solid UI methods that return a DOM element to return a list of values instead of a single DOM element, e.g. an expected return could be [HTMLElement, refresh] where type refresh = () => void.

We could add more elements as needed, and let each method define the list that they provide. But I think it would be valuable to standardize this list as well, to simply only return a list like I mentioned in last paragraph. This would be a bit similar to patterns like hooks in React (granted, the two elements are more like getter / setter, but I would argue element / refresh is close enough for this purpose), so it's not an unfamiliar pattern for front-end developers.

I think the [element, refresh]-pattern is enough for all components-methods in Solid UI, as they are connected to a global store (UI.store) or given a store explicitly that allows us to update the data model that refresh would require. I think all other properties should remain private to the component.

• Pros:
  • Clearer API design than what we have today
  • Simple
  • Future-proof
• Con:
  • Requires changing API
• Pro & Con (depending on how you see it):
  • Limited availability of extra API methods connected to element

I think this is the best solution if we want to avoid the anti-pattern of adding magic properties to DOM elements.

Wrapper

I used this approach when restructuring acl-control (which also led to similar patterns to AccessController, AccessGroups, and AddAgentButtons, which are all split up due the restructuring. The pattern is to have a render-method that returns the DOM element for the class. There can also be other elements contained within the class, of which all functionality are grouped by having them prefixed with render (e.g. AccessControll#renderTemporaryStatus). All other methods in the classes are not related to DOM-manipulation.

To accommodate refresh it could be renderRefresh, or maybe there's an exception just for that method.

• Pros
  • Clearer API design than what we have today
  • Future-proof
• Cons
  • Requires changing API
  • Requires a bit more setup
  • Users must access the element through another object

Proxy

Another approach is to wrap the DOM element in a Proxy. Deriving a solution that closely aligns to the example Using Proxy to make an infinitely chainable API, I think we could make a solution that would return the underlying DOM element on get and call refresh on set:

function someMethodThatReturnsTheProxy () {
  const element = document.createElement('div')
  const supportedMethods = {
    refresh: () => console.log('refresh')
  }
  return new Proxy(() => element, {
    get: (target, prop) => {
      return supportedMethods[prop]
    }
  })
}

const proxyForElement = someMethodThatReturnsTheProxy()
dom.appendChild(proxyForElement()) // returns the DOM element
proxyForElement.refresh() // logs 'refresh' in console

• Pros:
  • Clearer API design than what we have today
  • Minimal change to API
  • Future-proof
• Con
  • Requires changing API
  • I think it can be hard to discover the refresh-method
  • I think it reads a bit weird

Do note that the two latter cons might just be me not familiar with working with proxies, but I do not think this is the best solution.

Inizia esaminando i metodi che restituiscono il DOM e i file correlati indicati nella proposta: src/acl/acl-control.ts, access-controller.ts, access-groups.ts e add-agent-buttons.ts. Confronta i loro attuali pattern di aggiornamento e restituzione degli elementi con le tre alternative descritte. Questa operazione va eseguita solo dopo che i maintainer avranno scelto e approvato un’API standard e definito l’ambito della relativa migrazione.