Skip to content

Commit 1a3e50b

Browse files
fix: check srcset when hydrating to prevent needless requests (#8868)
--------- Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
1 parent d3d1fb5 commit 1a3e50b

File tree

4 files changed

+103
-4
lines changed

4 files changed

+103
-4
lines changed

.changeset/six-teachers-divide.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: check srcset when hydrating to prevent needless requests

packages/svelte/src/compiler/compile/render_dom/wrappers/Element/Attribute.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ export default class AttributeWrapper extends BaseAttributeWrapper {
6464
/** @type {boolean} */
6565
is_src;
6666

67+
/** @type {boolean} */
68+
is_srcset;
69+
6770
/** @type {boolean} */
6871
is_select_value_attribute;
6972

@@ -120,6 +123,9 @@ export default class AttributeWrapper extends BaseAttributeWrapper {
120123
this.is_src =
121124
this.name === 'src' &&
122125
(!this.parent.node.namespace || this.parent.node.namespace === namespaces.html);
126+
this.is_srcset =
127+
this.name === 'srcset' &&
128+
(!this.parent.node.namespace || this.parent.node.namespace === namespaces.html);
123129
this.should_cache = should_cache(this);
124130
}
125131

@@ -164,6 +170,11 @@ export default class AttributeWrapper extends BaseAttributeWrapper {
164170
b`if (!@src_url_equal(${element.var}.src, ${init})) ${method}(${element.var}, "${name}", ${this.last});`
165171
);
166172
updater = b`${method}(${element.var}, "${name}", ${should_cache ? this.last : value});`;
173+
} else if (this.is_srcset) {
174+
block.chunks.hydrate.push(
175+
b`if (!@srcset_url_equal(${element.var}, ${init})) ${method}(${element.var}, "${name}", ${this.last});`
176+
);
177+
updater = b`${method}(${element.var}, "${name}", ${should_cache ? this.last : value});`;
167178
} else if (property_name) {
168179
block.chunks.hydrate.push(b`${element.var}.${property_name} = ${init};`);
169180
updater = block.renderer.options.dev
@@ -403,7 +414,7 @@ Object.keys(attribute_lookup).forEach((name) => {
403414

404415
/** @param {AttributeWrapper} attribute */
405416
function should_cache(attribute) {
406-
return attribute.is_src || attribute.node.should_cache();
417+
return attribute.is_src || attribute.is_srcset || attribute.node.should_cache();
407418
}
408419
const regex_contains_checked_or_group = /checked|group/;
409420

packages/svelte/src/runtime/internal/utils.js

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,50 @@ export function safe_not_equal(a, b) {
6868

6969
let src_url_equal_anchor;
7070

71-
/** @returns {boolean} */
71+
/**
72+
* @param {string} element_src
73+
* @param {string} url
74+
* @returns {boolean}
75+
*/
7276
export function src_url_equal(element_src, url) {
77+
if (element_src === url) return true;
7378
if (!src_url_equal_anchor) {
7479
src_url_equal_anchor = document.createElement('a');
7580
}
81+
// This is actually faster than doing URL(..).href
7682
src_url_equal_anchor.href = url;
7783
return element_src === src_url_equal_anchor.href;
7884
}
7985

86+
/** @param {string} srcset */
87+
function split_srcset(srcset) {
88+
return srcset.split(',').map((src) => src.trim().split(' ').filter(Boolean));
89+
}
90+
91+
/**
92+
* @param {HTMLSourceElement | HTMLImageElement} element_srcset
93+
* @param {string} srcset
94+
* @returns {boolean}
95+
*/
96+
export function srcset_url_equal(element_srcset, srcset) {
97+
const element_urls = split_srcset(element_srcset.srcset);
98+
const urls = split_srcset(srcset);
99+
100+
return (
101+
urls.length === element_urls.length &&
102+
urls.every(
103+
([url, width], i) =>
104+
width === element_urls[i][1] &&
105+
// We need to test both ways because Vite will create an a full URL with
106+
// `new URL(asset, import.meta.url).href` for the client when `base: './'`, and the
107+
// relative URLs inside srcset are not automatically resolved to absolute URLs by
108+
// browsers (in contrast to img.src). This means both SSR and DOM code could
109+
// contain relative or absolute URLs.
110+
(src_url_equal(element_urls[i][0], url) || src_url_equal(url, element_urls[i][0]))
111+
)
112+
);
113+
}
114+
80115
/** @returns {boolean} */
81116
export function not_equal(a, b) {
82117
return a != a ? b == b : a !== b;

packages/svelte/test/utils/utils.test.js

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { assert, describe, it } from 'vitest';
1+
import { afterAll, assert, beforeAll, describe, it } from 'vitest';
22
import '../../src/compiler/compile/nodes/Slot.js'; // this needs to come first to force ESM to load things in a specific order to prevent circular dependency errors
33
import {
44
CONTENTEDITABLE_BINDINGS,
@@ -9,7 +9,7 @@ import {
99
} from '../../src/compiler/compile/utils/contenteditable.js';
1010
import get_name_from_filename from '../../src/compiler/compile/utils/get_name_from_filename.js';
1111
import { trim_end, trim_start } from '../../src/compiler/utils/trim.js';
12-
import { split_css_unit } from '../../src/runtime/internal/utils.js';
12+
import { split_css_unit, srcset_url_equal } from '../../src/runtime/internal/utils.js';
1313

1414
describe('utils', () => {
1515
describe('trim', () => {
@@ -116,4 +116,52 @@ describe('utils', () => {
116116
});
117117
});
118118
});
119+
120+
describe('srcset_url_equal', () => {
121+
function create_element(srcset) {
122+
return /** @type {HTMLImageElement} */ ({
123+
srcset
124+
});
125+
}
126+
127+
let old_document;
128+
129+
beforeAll(() => {
130+
const host = 'https://svelte.dev';
131+
let _href = '';
132+
old_document = global.document;
133+
global.document = /** @type {any} */ ({
134+
createElement: () =>
135+
/** @type {any} */ ({
136+
get href() {
137+
return _href;
138+
},
139+
set href(value) {
140+
_href = host + value;
141+
}
142+
})
143+
});
144+
});
145+
146+
afterAll(() => {
147+
global.document = old_document;
148+
});
149+
150+
it('should return true if urls are equal', () => {
151+
assert.ok(srcset_url_equal(create_element('a'), 'a'));
152+
assert.ok(srcset_url_equal(create_element('a 1x'), 'a 1x'));
153+
assert.ok(srcset_url_equal(create_element('a 1x, b 2x'), 'a 1x, b 2x'));
154+
assert.ok(srcset_url_equal(create_element('a 1x, b 2x'), 'a 1x, b 2x'));
155+
});
156+
157+
it('should return true if urls are equal (abs/rel URLs)', () => {
158+
assert.ok(srcset_url_equal(create_element('https://svelte.dev/a'), '/a'));
159+
assert.ok(srcset_url_equal(create_element('/a'), 'https://svelte.dev/a'));
160+
});
161+
162+
it('should return false if urls are different', () => {
163+
assert.notOk(srcset_url_equal(create_element('a 1x'), 'b 1x'));
164+
assert.notOk(srcset_url_equal(create_element('a 2x'), 'a 1x'));
165+
});
166+
});
119167
});

0 commit comments

Comments
 (0)