w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 667 forks source link

[css-highlight-api] Feedback: the global string-keyed registry makes usage very hard #11095

Open nicolo-ribaudo opened 2 weeks ago

nicolo-ribaudo commented 2 weeks ago

Example use case: I have a contenteditable div that marks as ::highlight(has-x) all the words that contain x.

The simplest way of doing it is:

function highlightWords(div) {
  const highlight = new Highlight();

  for (const child of div.childNodes) {
    if (child.nodeType !== Node.TEXT_NODE) continue;

    const regexp = /\w*x\w*/g;
    let match;
    while (match = regexp.exec(child.textContent)) {
      const range = document.createRange();
      range.setStart(node, match.index);
      range.setEnd(node, match[0].length + match.index);
      highlight.add(range);
    }
  }

  CSS.highlights.set("has-x", highlight);
}

highlightWords(myElement);
myElement.addEventListener("input", () => highlightWords(myElement));

This works perfectly as long as I only call this function on a single element.

One day I decide that actually, I need this behavior on two different elements on my page, so I try doing this:

highlightWords(myElement1);
myElement1.addEventListener("input", () => highlightWords(myElement1));

highlightWords(myElement2);
myElement1.addEventListener("input", () => highlightWords(myElement2));

Except that this doesn't work at all: the highlighting logic over myElement1 and myElement2 fight over control of the has-x entry in the global CSS.highlights registry, and delete each other's highlights as they get updated.

To make it work, I need to instead update the logic in highlightWords to never overwrite an existing entry in the CSS.highlights map. Instead, it must check through all the ranges in the entry and only remove/add its own, and then if the map ends up being empty it can remove it.

The global registry introduces a synchronization point between different parts of the page that is easier to get wrong than it's to get right, and if you are a library author (for example, publishing the logic above in a custom element) you won't notice unless you explicitly test multiple instances on the same page.


There are multiple ways that the API can be changed to make it difficult to do the wrong thing.

Do not allow reading Highlight objects from the global registry, and do not allow removing Highlight objects that you do not control:

This keeps the synchronization complexity, but forces developers to think about it.

Allow registering multiple Highlights with the same name:

Similar to above, except that instead of it makes conflicts work well together instead of overwriting each other

Allow using local registries

Allow the function above to create its own highlights registry by doing something like const highlights = new HighlightsRegistry(document). Different registries can declare the same name without conflicting with each other. The example above would become

const registry1 = new HighlightsRegistry(document);
highlightWords(registry1, myElement1);
myElement1.addEventListener("input", () => highlightWords(registry1, myElement1));

const registry2 = new HighlightsRegistry(document);
highlightWords(registry2, myElement2);
myElement1.addEventListener("input", () => highlightWords(registry2, myElement2));

Get rid of the registry

Highlight objects themselves could be automatically "registered". You'd have to create them with

const highlight = new Highlight(document, name);
highlight.addRange(...);
highlight.addRange(...);

and then you can "unregister" them with highlight.removeAllRanges().

schenney-chromium commented 2 weeks ago

I agree that this is a significant ergonomic problem with the highlight registry. Of the suggestions (thanks for the proposals!) the first seems best in the sense that it is clearly possible to implement efficiently.

The other proposals all have significant downside. Some still require the app to manage the ranges associated with a highlight, in which case the first proposal addresses the issue. Others complicate the design of the Highlight or HighlightRegistry object but still leave the app having to control the logic mapping highlight names to ranges.

nicolo-ribaudo commented 2 weeks ago

For what is worth, the for project that I was working on when I opened this issue I ended up writing my own LocalHighlightRegistry that abstracts away the synchronization across different usages: https://jsr.io/@nic/local-highlight-registry.

It's working well for me, I encourage you and others to try it :)