whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.16k stars 2.69k forks source link

Improve API of `insertAdjacent*()` methods #10122

Open LeaVerou opened 9 months ago

LeaVerou commented 9 months ago

What problem are you trying to solve?

insertAdjacentHTML() is fantastic for templating. It bridged an old usability cliff where authors had to either give up working with HTML strings, or do a lot of painful wrangling with innerHTML if they only wanted to insert an HTML fragment inside or relative to another element, rather than replace the entire contents of an element.

However, due to its origins, the API is …let’s say suboptimal and clumsy. The position arguments are excessively long and inconsistent with related DOM methods (e.g. child.before() but "beforebegin"), and there’s not even a default for the first argument, so authors are forced to specify them on every call.

Furthermore, we also have insertAdjacentElement() and insertAdjacentText() methods, which are simply worse APIs for before() / after() / append() / prepend(), presumably included for compat.

How would you solve it?

Option 1: Overloading insertAdjacent*() methods

  1. Deprecate existing position strings, and instead move to:
    • "before" instead of "beforebegin"
    • "after" instead of "afterend"
    • "start" (or "prepend"?) instead of "afterbegin"
    • "end" (or "append"?) instead of "beforeend"
  2. Default position to "end"

Option 1b: Instead of overloading the signature with a one argument signature to default position, introduce a dictionary argument with position and html keys (or text or node for the other two).

Pros:

Cons:

Option 2: Introducing new <position>HTML() methods

This would introduce new HTML methods like appendHTML(), prependHTML(), beforeHTML(), afterHTML()

Not a huge fan of this approach, as it increases the API surface significantly, and having different methods that do related but different things is an antipattern. However, it's the only one that doesn't involve overloading existing methods.

Option 3: Overload element.append(), element.prepend(), node.before(), node.after()

Since these already handle strings as text nodes, overloading wouldn't work. However, a dictionary overload with an html key still could. We probably want to be able to combine HTML strings with elements and text nodes, so this would still accept multiple arguments, each of which can be a dictionary.

Pros:

Cons:

Option 4: Single node.insert(...content) method to rule them all

This would basically encompass all insertAdjacent* PLUS functionality in a single method. content could be either a string (which would create a text node), a node, or a dictionary with the following structure:

{
    html?: string,
    text?: string
    node?: string,
    [position = "end"]: "before" | "after" | "start" | "end"
}

The method would ideally be available on Node and would error if start | end are used on non-elements.

Pros:

Cons:

annevk commented 9 months ago

HTML defines the parser APIs so moving this issue there. https://github.com/WICG/sanitizer-api/issues/184#issuecomment-1378711488 still seems the most reasonable to me (your option 2) although now we'd have to name these beforeHTMLUnsafe(), etc. It's quite a few additional methods, but adding methods doesn't have a significant cost. And it's often clearer to have several well-named methods than an overloaded one.

zcorpan commented 9 months ago

The legacy HTML fragment parser APIs don't support DSD and don't have Sanitizer API support. They also parse as HTML or XML depending on the "HTML document" bit on the document, and setHTML() and setHTMLUnsafe() always parse as HTML. (If we want XML variants in the future, we can add setXMLUnsafe() etc.) So possible axes are:

I think option 1 is not a sufficient improvement to be worthwhile, it would still be a clunky API and it might be hard to add Sanitizer API in a consistent way.

Option 3 would make existing methods XSS sinks.