From 1a3e50b6b7ebb4628446c4f852880e52c14213cd Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 30 Jun 2023 10:57:40 +0200 Subject: [PATCH] fix: check srcset when hydrating to prevent needless requests (#8868) --------- Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> --- .changeset/six-teachers-divide.md | 5 ++ .../render_dom/wrappers/Element/Attribute.js | 13 ++++- packages/svelte/src/runtime/internal/utils.js | 37 ++++++++++++- packages/svelte/test/utils/utils.test.js | 52 ++++++++++++++++++- 4 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 .changeset/six-teachers-divide.md diff --git a/.changeset/six-teachers-divide.md b/.changeset/six-teachers-divide.md new file mode 100644 index 000000000000..a11f257fefe9 --- /dev/null +++ b/.changeset/six-teachers-divide.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: check srcset when hydrating to prevent needless requests diff --git a/packages/svelte/src/compiler/compile/render_dom/wrappers/Element/Attribute.js b/packages/svelte/src/compiler/compile/render_dom/wrappers/Element/Attribute.js index d64626137725..0459b55b0357 100644 --- a/packages/svelte/src/compiler/compile/render_dom/wrappers/Element/Attribute.js +++ b/packages/svelte/src/compiler/compile/render_dom/wrappers/Element/Attribute.js @@ -64,6 +64,9 @@ export default class AttributeWrapper extends BaseAttributeWrapper { /** @type {boolean} */ is_src; + /** @type {boolean} */ + is_srcset; + /** @type {boolean} */ is_select_value_attribute; @@ -120,6 +123,9 @@ export default class AttributeWrapper extends BaseAttributeWrapper { this.is_src = this.name === 'src' && (!this.parent.node.namespace || this.parent.node.namespace === namespaces.html); + this.is_srcset = + this.name === 'srcset' && + (!this.parent.node.namespace || this.parent.node.namespace === namespaces.html); this.should_cache = should_cache(this); } @@ -164,6 +170,11 @@ export default class AttributeWrapper extends BaseAttributeWrapper { b`if (!@src_url_equal(${element.var}.src, ${init})) ${method}(${element.var}, "${name}", ${this.last});` ); updater = b`${method}(${element.var}, "${name}", ${should_cache ? this.last : value});`; + } else if (this.is_srcset) { + block.chunks.hydrate.push( + b`if (!@srcset_url_equal(${element.var}, ${init})) ${method}(${element.var}, "${name}", ${this.last});` + ); + updater = b`${method}(${element.var}, "${name}", ${should_cache ? this.last : value});`; } else if (property_name) { block.chunks.hydrate.push(b`${element.var}.${property_name} = ${init};`); updater = block.renderer.options.dev @@ -403,7 +414,7 @@ Object.keys(attribute_lookup).forEach((name) => { /** @param {AttributeWrapper} attribute */ function should_cache(attribute) { - return attribute.is_src || attribute.node.should_cache(); + return attribute.is_src || attribute.is_srcset || attribute.node.should_cache(); } const regex_contains_checked_or_group = /checked|group/; diff --git a/packages/svelte/src/runtime/internal/utils.js b/packages/svelte/src/runtime/internal/utils.js index df97d49598ab..190c4534c626 100644 --- a/packages/svelte/src/runtime/internal/utils.js +++ b/packages/svelte/src/runtime/internal/utils.js @@ -68,15 +68,50 @@ export function safe_not_equal(a, b) { let src_url_equal_anchor; -/** @returns {boolean} */ +/** + * @param {string} element_src + * @param {string} url + * @returns {boolean} + */ export function src_url_equal(element_src, url) { + if (element_src === url) return true; if (!src_url_equal_anchor) { src_url_equal_anchor = document.createElement('a'); } + // This is actually faster than doing URL(..).href src_url_equal_anchor.href = url; return element_src === src_url_equal_anchor.href; } +/** @param {string} srcset */ +function split_srcset(srcset) { + return srcset.split(',').map((src) => src.trim().split(' ').filter(Boolean)); +} + +/** + * @param {HTMLSourceElement | HTMLImageElement} element_srcset + * @param {string} srcset + * @returns {boolean} + */ +export function srcset_url_equal(element_srcset, srcset) { + const element_urls = split_srcset(element_srcset.srcset); + const urls = split_srcset(srcset); + + return ( + urls.length === element_urls.length && + urls.every( + ([url, width], i) => + width === element_urls[i][1] && + // We need to test both ways because Vite will create an a full URL with + // `new URL(asset, import.meta.url).href` for the client when `base: './'`, and the + // relative URLs inside srcset are not automatically resolved to absolute URLs by + // browsers (in contrast to img.src). This means both SSR and DOM code could + // contain relative or absolute URLs. + (src_url_equal(element_urls[i][0], url) || src_url_equal(url, element_urls[i][0])) + ) + ); +} + /** @returns {boolean} */ export function not_equal(a, b) { return a != a ? b == b : a !== b; diff --git a/packages/svelte/test/utils/utils.test.js b/packages/svelte/test/utils/utils.test.js index cbdba7db320a..301361ea3d56 100644 --- a/packages/svelte/test/utils/utils.test.js +++ b/packages/svelte/test/utils/utils.test.js @@ -1,4 +1,4 @@ -import { assert, describe, it } from 'vitest'; +import { afterAll, assert, beforeAll, describe, it } from 'vitest'; 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 import { CONTENTEDITABLE_BINDINGS, @@ -9,7 +9,7 @@ import { } from '../../src/compiler/compile/utils/contenteditable.js'; import get_name_from_filename from '../../src/compiler/compile/utils/get_name_from_filename.js'; import { trim_end, trim_start } from '../../src/compiler/utils/trim.js'; -import { split_css_unit } from '../../src/runtime/internal/utils.js'; +import { split_css_unit, srcset_url_equal } from '../../src/runtime/internal/utils.js'; describe('utils', () => { describe('trim', () => { @@ -116,4 +116,52 @@ describe('utils', () => { }); }); }); + + describe('srcset_url_equal', () => { + function create_element(srcset) { + return /** @type {HTMLImageElement} */ ({ + srcset + }); + } + + let old_document; + + beforeAll(() => { + const host = 'https://svelte.dev'; + let _href = ''; + old_document = global.document; + global.document = /** @type {any} */ ({ + createElement: () => + /** @type {any} */ ({ + get href() { + return _href; + }, + set href(value) { + _href = host + value; + } + }) + }); + }); + + afterAll(() => { + global.document = old_document; + }); + + it('should return true if urls are equal', () => { + assert.ok(srcset_url_equal(create_element('a'), 'a')); + assert.ok(srcset_url_equal(create_element('a 1x'), 'a 1x')); + assert.ok(srcset_url_equal(create_element('a 1x, b 2x'), 'a 1x, b 2x')); + assert.ok(srcset_url_equal(create_element('a 1x, b 2x'), 'a 1x, b 2x')); + }); + + it('should return true if urls are equal (abs/rel URLs)', () => { + assert.ok(srcset_url_equal(create_element('https://svelte.dev/a'), '/a')); + assert.ok(srcset_url_equal(create_element('/a'), 'https://svelte.dev/a')); + }); + + it('should return false if urls are different', () => { + assert.notOk(srcset_url_equal(create_element('a 1x'), 'b 1x')); + assert.notOk(srcset_url_equal(create_element('a 2x'), 'a 1x')); + }); + }); });