Skip to content

Commit 180e315

Browse files
brainkimclaude
andcommitted
Fix URL property comparison for src and href attributes
The DOM property for src and href returns the resolved absolute URL, while the prop value may be a relative URL. This caused unnecessary DOM updates on every render when using relative URLs like "/path". Now we resolve the prop value using new URL(value, element.baseURI) before comparing to the DOM value, maintaining the "DOM as source of truth" philosophy while correctly handling URL normalization. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 377ff44 commit 180e315

File tree

2 files changed

+130
-1
lines changed

2 files changed

+130
-1
lines changed

src/dom.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,24 @@ export const adapter: Partial<RenderAdapter<Node, string, Element>> = {
462462
) &&
463463
isWritableProperty(element, name)
464464
) {
465-
if ((element as any)[name] !== value || oldValue === undefined) {
465+
// For URL properties like src and href, the DOM property returns the
466+
// resolved absolute URL. We need to resolve the prop value the same way
467+
// to compare correctly.
468+
let domValue = (element as any)[name];
469+
let propValue = value;
470+
if (
471+
(name === "src" || name === "href") &&
472+
typeof value === "string" &&
473+
typeof domValue === "string"
474+
) {
475+
try {
476+
propValue = new URL(value, element.baseURI).href;
477+
} catch {
478+
// Invalid URL, use original value for comparison
479+
}
480+
}
481+
482+
if (propValue !== domValue || oldValue === undefined) {
466483
if (
467484
isHydrating &&
468485
typeof (element as any)[name] === "string" &&

test/dom.tsx

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,4 +777,116 @@ test("object classnames object to string transition", () => {
777777
Assert.not.ok(element.classList.contains("another-class"));
778778
});
779779

780+
test("relative src should not cause unnecessary updates", () => {
781+
// Track how many times src property is set
782+
let srcSetCount = 0;
783+
const originalDescriptor = Object.getOwnPropertyDescriptor(
784+
HTMLIFrameElement.prototype,
785+
"src",
786+
)!;
787+
Object.defineProperty(HTMLIFrameElement.prototype, "src", {
788+
get: originalDescriptor.get,
789+
set(value) {
790+
srcSetCount++;
791+
originalDescriptor.set!.call(this, value);
792+
},
793+
configurable: true,
794+
});
795+
796+
try {
797+
// Initial render
798+
renderer.render(<iframe src="/test-path" />, document.body);
799+
Assert.is(srcSetCount, 1, "src should be set once on initial render");
800+
801+
// Re-render with same src
802+
renderer.render(<iframe src="/test-path" />, document.body);
803+
Assert.is(srcSetCount, 1, "src should not be set again when unchanged");
804+
805+
// Re-render with different src
806+
renderer.render(<iframe src="/different-path" />, document.body);
807+
Assert.is(srcSetCount, 2, "src should be set when changed");
808+
} finally {
809+
// Restore original property
810+
Object.defineProperty(
811+
HTMLIFrameElement.prototype,
812+
"src",
813+
originalDescriptor,
814+
);
815+
}
816+
});
817+
818+
test("relative href should not cause unnecessary updates", () => {
819+
// Track how many times href property is set
820+
let hrefSetCount = 0;
821+
const originalDescriptor = Object.getOwnPropertyDescriptor(
822+
HTMLAnchorElement.prototype,
823+
"href",
824+
)!;
825+
Object.defineProperty(HTMLAnchorElement.prototype, "href", {
826+
get: originalDescriptor.get,
827+
set(value) {
828+
hrefSetCount++;
829+
originalDescriptor.set!.call(this, value);
830+
},
831+
configurable: true,
832+
});
833+
834+
try {
835+
// Initial render
836+
renderer.render(<a href="/test-link">Link</a>, document.body);
837+
Assert.is(hrefSetCount, 1, "href should be set once on initial render");
838+
839+
// Re-render with same href
840+
renderer.render(<a href="/test-link">Link</a>, document.body);
841+
Assert.is(hrefSetCount, 1, "href should not be set again when unchanged");
842+
843+
// Re-render with different href
844+
renderer.render(<a href="/different-link">Link</a>, document.body);
845+
Assert.is(hrefSetCount, 2, "href should be set when changed");
846+
} finally {
847+
// Restore original property
848+
Object.defineProperty(
849+
HTMLAnchorElement.prototype,
850+
"href",
851+
originalDescriptor,
852+
);
853+
}
854+
});
855+
856+
test("absolute URLs should work correctly for src", () => {
857+
let srcSetCount = 0;
858+
const originalDescriptor = Object.getOwnPropertyDescriptor(
859+
HTMLIFrameElement.prototype,
860+
"src",
861+
)!;
862+
Object.defineProperty(HTMLIFrameElement.prototype, "src", {
863+
get: originalDescriptor.get,
864+
set(value) {
865+
srcSetCount++;
866+
originalDescriptor.set!.call(this, value);
867+
},
868+
configurable: true,
869+
});
870+
871+
try {
872+
// Initial render with absolute URL
873+
renderer.render(<iframe src="https://example.com/page" />, document.body);
874+
Assert.is(srcSetCount, 1);
875+
876+
// Re-render with same absolute URL
877+
renderer.render(<iframe src="https://example.com/page" />, document.body);
878+
Assert.is(
879+
srcSetCount,
880+
1,
881+
"absolute src should not be set again when unchanged",
882+
);
883+
} finally {
884+
Object.defineProperty(
885+
HTMLIFrameElement.prototype,
886+
"src",
887+
originalDescriptor,
888+
);
889+
}
890+
});
891+
780892
test.run();

0 commit comments

Comments
 (0)