Skip to content

Commit

Permalink
fix: collect all necessary setters of html elements (#11371)
Browse files Browse the repository at this point in the history
When spreading attributes, the setters of the element are checked. If they contain the key in question, it's set via that setter. For certain setters on certain elements this didn't work because the element prototype was not HTMLElement, rather a descendant of that (for example HTMLDivElement), which meant that only the setters of the descendant, not the superclass were taken into account. This fixes that by walking up the prototype chain until we find the Element prototype.

fixes #11179
  • Loading branch information
dummdidumm authored Apr 29, 2024
1 parent cd25065 commit eb7e32c
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/blue-lemons-wait.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: collect all necessary setters of html elements when spreading attributes
18 changes: 12 additions & 6 deletions packages/svelte/src/internal/client/dom/elements/attributes.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { DEV } from 'esm-env';
import { hydrating } from '../hydration.js';
import { get_descriptors, map_get, map_set, object_assign } from '../../utils.js';
import { get_descriptors, get_prototype_of, map_get, map_set } from '../../utils.js';
import { AttributeAliases, DelegatedEvents, namespace_svg } from '../../../../constants.js';
import { delegate } from './events.js';
import { autofocus } from './misc.js';
Expand Down Expand Up @@ -241,14 +241,20 @@ var setters_cache = new Map();
function get_setters(element) {
/** @type {string[]} */
var setters = [];
var descriptors;
var proto = get_prototype_of(element);

// @ts-expect-error
var descriptors = get_descriptors(element.__proto__);
// Stop at Element, from there on there's only unnecessary setters we're not interested in
while (proto.constructor.name !== 'Element') {
descriptors = get_descriptors(proto);

for (var key in descriptors) {
if (descriptors[key].set && !always_set_through_set_attribute.includes(key)) {
setters.push(key);
for (var key in descriptors) {
if (descriptors[key].set && !always_set_through_set_attribute.includes(key)) {
setters.push(key);
}
}

proto = get_prototype_of(proto);
}

return setters;
Expand Down
5 changes: 1 addition & 4 deletions packages/svelte/tests/runtime-browser/test-ssr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ export async function run_ssr_test(
test_dir: string
) {
try {
await compile_directory(test_dir, 'server', {
...config.compileOptions,
runes: test_dir.includes('runtime-runes')
});
await compile_directory(test_dir, 'server', config.compileOptions);

const Component = (await import(`${test_dir}/_output/server/main.svelte.js`)).default;
const { html } = render(Component, { props: config.props || {} });
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { test } from '../../test';

export default test({
async test({ target, assert }) {
const div = target.querySelector('div');
const btn = target.querySelector('button');

assert.equal(div?.hidden, true);

await btn?.click();
assert.equal(div?.hidden, false);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<script>
let hidden = $state(true);
const restProps = {
id: '123'
}
</script>

<button onclick={() => hidden = !hidden}>
toggle hidden
</button>

<div {...restProps} hidden={hidden}>
hello world (with spread attrs)
</div>

0 comments on commit eb7e32c

Please sign in to comment.