Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: src and loading in lazy loaded images returns right value immediately #15141

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/friendly-crews-sin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: `src` and `loading` in lazy loaded images returns right value immediately
59 changes: 51 additions & 8 deletions packages/svelte/src/internal/client/dom/elements/attributes.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import { DEV } from 'esm-env';
import { hydrating } from '../hydration.js';
import { get_descriptors, get_prototype_of } from '../../../shared/utils.js';
import { create_event, delegate } from './events.js';
import { add_form_reset_listener, autofocus } from './misc.js';
import * as w from '../../warnings.js';
import { LOADING_ATTR_SYMBOL } from '../../constants.js';
import { queue_idle_task } from '../task.js';
import { is_capture_event, is_delegated, normalize_attribute } from '../../../../utils.js';
import { clsx } from '../../../shared/attributes.js';
import { define_property, get_descriptors, get_prototype_of } from '../../../shared/utils.js';
import { LOADING_ATTR_SYMBOL } from '../../constants.js';
import {
active_effect,
active_reaction,
set_active_effect,
set_active_reaction
} from '../../runtime.js';
import { clsx } from '../../../shared/attributes.js';
import * as w from '../../warnings.js';
import { hydrating } from '../hydration.js';
import { queue_idle_task } from '../task.js';
import { create_event, delegate } from './events.js';
import { add_form_reset_listener, autofocus } from './misc.js';

/**
* The value/checked attribute in the template actually corresponds to the defaultValue property, so we need
Expand Down Expand Up @@ -515,11 +515,54 @@ export function handle_lazy_img(element) {
// templates.
if (!hydrating && element.loading === 'lazy') {
var src = element.src;
var loading = element.loading;
// @ts-expect-error
element[LOADING_ATTR_SYMBOL] = null;
element.loading = 'eager';
element.removeAttribute('src');

// since we are removing src before the first animation frame if someone
// access src or loading on mount or in an action they will have the wrong value
// so we define those two properties so that if accessed through JS it will
// return the correct value. Since we can't undo this we also mimic the behavior
// of setting the attribute in the setter
define_property(element, 'src', {
get() {
return src;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't correct? It doesn't take into account relative pathing AFAIK which differs from the attribute value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console.log(img.getAttribute("src")); // "images/foo.png"
console.log(img.src);                 // "http://example.com/path/images/foo.png"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh...I knew something was gonna bit me 😁 I'll check it I can find a fix

},
set(new_src) {
element.setAttribute('src', new_src);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the same issue with relative paths applies here too

src = new_src;
},
configurable: true,
enumerable: true
});
define_property(element, 'loading', {
get() {
return loading;
},
set(new_loading) {
element.setAttribute('loading', new_loading);
loading = new_loading;
},
configurable: true,
enumerable: true
});

// same story as above with getAttribute...we fake it until we restore the
// old `setAttribute` value after the first animation frame
const old_get_attribute = element.getAttribute;
element.getAttribute = (name) => {
if (name === 'src') {
return src;
}
if (name === 'loading') {
return loading;
}
return old_get_attribute.call(element, name);
};
requestAnimationFrame(() => {
element.getAttribute = old_get_attribute;
// @ts-expect-error
if (element[LOADING_ATTR_SYMBOL] !== 'eager') {
element.loading = 'lazy';
Expand Down