Skip to content

Commit

Permalink
fix: check srcset when hydrating to prevent needless requests (#8868)
Browse files Browse the repository at this point in the history
---------

Co-authored-by: Ben McCann <[email protected]>
  • Loading branch information
dummdidumm and benmccann authored Jun 30, 2023
1 parent d3d1fb5 commit 1a3e50b
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/six-teachers-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: check srcset when hydrating to prevent needless requests
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ export default class AttributeWrapper extends BaseAttributeWrapper {
/** @type {boolean} */
is_src;

/** @type {boolean} */
is_srcset;

/** @type {boolean} */
is_select_value_attribute;

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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/;

Expand Down
37 changes: 36 additions & 1 deletion packages/svelte/src/runtime/internal/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
52 changes: 50 additions & 2 deletions packages/svelte/test/utils/utils.test.js
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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'));
});
});
});

0 comments on commit 1a3e50b

Please sign in to comment.