chore: check namespace inside set attributes

[Rich-Harris]

Possible alternative to #15413 — results in more compact compiler output — this...

<svelte:element this={'div'} fooBar={42}></svelte:element>

...becomes this...

$.element(node, () => 'div', false, ($$element, $$anchor) => {
  $.set_attributes($$element, null, { fooBar: 42 });
});

...instead of this (on main)...

$.element(node, () => 'div', false, ($$element, $$anchor) => {
  $.set_attributes($$element, null, { fooBar: 42 }, undefined, $$element.namespaceURI === $.NAMESPACE_SVG, $$element.nodeName.includes('-'));
});

...or this (in #15413):

$.element(node, () => 'div', false, (
  $$element,
  $$anchor,
  $$preserve_attribute_case,
  $$is_custom_element
) => {
  $.set_attributes($$element, null, { fooBar: 42 }, undefined, $$preserve_attribute_case, $$is_custom_element);
});

The trade-off is that we need to determine is_custom_element and preserve_attribute_case every time set_attributes runs. But I think it's worth it for the smaller output and somewhat simpler code

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.
• If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: d7caac0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-svelte-15443-svelte.vercel.app/

[github-actions]

Playground

pnpm add https://pkg.pr.new/svelte@15443

[adiguba]

Note that this will make the code longer for regular elements, as it currently use the default value of these params.

Also, the generated code on the PR 15413 is longuest because it use long attribute names, but that should be reduced by minifier.

Perhaps another alternative is to use an unique flag to group boths values into one :

$.element(node, () => 'div', false, (
  $$element,
  $$anchor,
  $$flag
) => {
  $.set_attributes($$element, null, { fooBar: 42 }, undefined, $$flag);
});

With something like that inside set_attributes() :

  const preserve_attribute_case = flags & PRESERVE_ATTRIBUTE_CASE;
  const is_custom_element = flags & CUSTOM_ELEMENT;

[trueadm]

Can't we cache this in the internal __attributes object somehow? The namespace check is actually quite expensive it seems benchmarking.

[Rich-Harris]

good idea, done

[Rich-Harris]

The flags idea is smart, though doing it inside set_attributes still feels a little neater. I just pushed a change that means we only do the work once at most

[adiguba]

The flags can have a default value or be determined at compile time in most case like actually for regular element

With this PR we will now check that at runtime instead of compile time...

[trueadm]

This is a lot better now, having to do that check for every attribute that has properties would have had an impact on performance.

However, I think passing in a single flag would also work well for the simple element case, the flag is to say that we need a runtime check for those two properties. In the vast majority of cases these never need to be checked for. So if we optimise for that case then we likely never have to do any work here at all.

[trueadm]

It would be good to get an idea from our website or something how many times this is normally called. If it's a lot then we probably want to avoid it for the simple element case – which is the vast majority of elements.

[Rich-Harris]

For svelte.dev it's 147, for svelte.dev/playground it's 260, for svelte.dev/tutorial it's 302. If you interact with the search box it will increase by a few hundred. Obviously if you had a very large list then it would be more, but the upshot is that we're typically talking about hundreds of operations.

I don't know if this is a valid microbenchmark, but on this M1 MBP I can run the check 10,000 times (i.e. run(1e4) before it takes more than a single millisecond:

function run(i = 1e6) {
  console.time('test');
  let div = document.createElement('div');
  while (i--) {
    div.nodeName.includes('-');
    div.namespaceURI === 'http://www.w3.org/1999/xhtml';
  }
  console.timeEnd('test');
}

So... it's measurable, but seems pretty negligible. I don't know, what do you think?

[trueadm]

What if you create the elements and attach them to the DOM and then test the lookup cost? It seems more realistic to check attached elements and to remove the cost of creating the DOM element from the benchmark.

[Rich-Harris]

Makes no real difference

function run(i = 1e6) {
  let div = document.createElement('div');
  document.body.append(div);
  console.time('test');
  while (i--) {
    div.nodeName.includes('-');
    div.namespaceURI === 'http://www.w3.org/1999/xhtml';
  }
  console.timeEnd('test');
}

[trueadm]

I mean:

function run(i = 1e6) {
  const els = Array(1e6);
  let s = i;
  while (s--) {
    let div = document.createElement('div');
    document.body.append(div);
    els[s] = div;
  }
  console.time('test');
  while (i--) {
    const div = els[i];
    div.nodeName.includes('-');
    div.namespaceURI === 'http://www.w3.org/1999/xhtml';
  }
  console.timeEnd('test');
}

[trueadm]

Still looks good though, let's ship it and if needed we can revisit and optimise in the future.

[Rich-Harris]

Try it! The results are basically the same. Though the code above isn't correct, it should be this:

function run(n = 1e6) {
  const els = Array(n);
  let i = n;
  while (i--) {
    let div = document.createElement('div');
    document.body.append(div);
    els[i] = div;
  }
  console.time('test');
  i = n;
  while (i--) {
    const div = els[i];
    div.nodeName.includes('-');
    div.namespaceURI === 'http://www.w3.org/1999/xhtml';
  }
  console.timeEnd('test');
}