Skip to content

Commit fbf69a9

Browse files
authored
Handle aria and data attributes consistently (#782)
1 parent c0b06ff commit fbf69a9

File tree

3 files changed

+77
-1
lines changed

3 files changed

+77
-1
lines changed

.changeset/five-ligers-hear.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@preact/signals": patch
3+
---
4+
5+
Ensure aria/data attributes stick around when going back to an empty string

packages/preact/src/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,9 @@ function createPropUpdater(
324324
if (setAsProperty) {
325325
// @ts-ignore-next-line silly
326326
dom[prop] = value;
327-
} else if (value) {
327+
// Match Preact's attribute handling: data-* and aria-* attributes
328+
// https://github.com/preactjs/preact/blob/main/src/diff/props.js#L132
329+
} else if (value != null && (value !== false || prop[4] === "-")) {
328330
dom.setAttribute(prop, value);
329331
} else {
330332
dom.removeAttribute(prop);

packages/preact/test/index.test.tsx

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,75 @@ describe("@preact/signals", () => {
728728
s.value = "scale(1, 2)";
729729
});
730730
});
731+
732+
// https://github.com/preactjs/signals/issues/781
733+
it("should handle empty string data-* attributes consistently", async () => {
734+
const s = signal("");
735+
const spy = sinon.spy();
736+
737+
function App() {
738+
spy();
739+
// @ts-ignore
740+
return <div data-text={s} />;
741+
}
742+
743+
render(<App />, scratch);
744+
spy.resetHistory();
745+
746+
const div = scratch.firstChild as HTMLDivElement;
747+
748+
expect(div.hasAttribute("data-text")).to.equal(true);
749+
expect(div.getAttribute("data-text")).to.equal("");
750+
751+
act(() => {
752+
s.value = "test";
753+
});
754+
755+
expect(div.getAttribute("data-text")).to.equal("test");
756+
expect(spy).not.to.have.been.called;
757+
758+
act(() => {
759+
s.value = "";
760+
});
761+
762+
expect(div.hasAttribute("data-text")).to.equal(true);
763+
expect(div.getAttribute("data-text")).to.equal("");
764+
expect(spy).not.to.have.been.called;
765+
});
766+
767+
it("should handle empty string on regular attributes consistently", async () => {
768+
const s = signal("");
769+
const spy = sinon.spy();
770+
771+
function App() {
772+
spy();
773+
// @ts-ignore
774+
return <div title={s} />;
775+
}
776+
777+
render(<App />, scratch);
778+
spy.resetHistory();
779+
780+
const div = scratch.firstChild as HTMLDivElement;
781+
782+
expect(div.hasAttribute("title")).to.equal(true);
783+
expect(div.getAttribute("title")).to.equal("");
784+
785+
act(() => {
786+
s.value = "test";
787+
});
788+
789+
expect(div.getAttribute("title")).to.equal("test");
790+
expect(spy).not.to.have.been.called;
791+
792+
act(() => {
793+
s.value = "";
794+
});
795+
796+
expect(div.hasAttribute("title")).to.equal(true);
797+
expect(div.getAttribute("title")).to.equal("");
798+
expect(spy).not.to.have.been.called;
799+
});
731800
});
732801

733802
describe("hooks mixed with signals", () => {

0 commit comments

Comments
 (0)