Skip to content

Commit 58314b7

Browse files
janechuCopilot
andcommitted
refactor(fast-html): use {{foobar}} binding (no dashes) in AttributeMap fixture
Template bindings should not use dashes. The attribute name can still have dashes (e.g. foo-bar on the element), but the binding itself uses foobar. Update fixture template, main.ts, and all spec descriptions accordingly. Clarify removeDashes JSDoc to reflect the design intent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f1482e3 commit 58314b7

File tree

4 files changed

+23
-20
lines changed

4 files changed

+23
-20
lines changed

packages/fast-html/src/components/attribute-map.spec.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ test.describe("AttributeMap", async () => {
2020
expect(hasFooAccessor).toBeTruthy();
2121
});
2222

23-
test("should define @attr for a property with dashes removed", async ({ page }) => {
23+
test("should define @attr for a foobar property", async ({ page }) => {
2424
const element = page.locator("attribute-map-test-element");
2525

2626
const hasFoobarAccessor = await element.evaluate(node => {
@@ -34,16 +34,16 @@ test.describe("AttributeMap", async () => {
3434
expect(hasFoobarAccessor).toBeTruthy();
3535
});
3636

37-
test("should remove dashes from attribute name to form property name", async ({
37+
test("should define @attr for a foobar property using the same attribute name", async ({
3838
page,
3939
}) => {
4040
const element = page.locator("attribute-map-test-element");
4141

42-
// Setting the dash-case attribute should update the dashes-removed property
43-
await element.evaluate(node => node.setAttribute("foo-bar", "dash-case-test"));
42+
// Setting the foobar attribute should update the foobar property
43+
await element.evaluate(node => node.setAttribute("foobar", "foobar-test"));
4444
const propValue = await element.evaluate(node => (node as any).foobar);
4545

46-
expect(propValue).toBe("dash-case-test");
46+
expect(propValue).toBe("foobar-test");
4747
});
4848

4949
test("should not define @attr for event handler methods", async ({ page }) => {
@@ -79,12 +79,12 @@ test.describe("AttributeMap", async () => {
7979
await expect(page.locator(".foo-value")).toHaveText("attr-value");
8080
});
8181

82-
test("should update template when dash-case attribute is set via setAttribute", async ({
82+
test("should update template when foobar attribute is set via setAttribute", async ({
8383
page,
8484
}) => {
8585
const element = page.locator("attribute-map-test-element");
8686

87-
await element.evaluate(node => node.setAttribute("foo-bar", "bar-attr-value"));
87+
await element.evaluate(node => node.setAttribute("foobar", "bar-attr-value"));
8888

8989
await expect(page.locator(".foo-bar-value")).toHaveText("bar-attr-value");
9090
});
@@ -115,13 +115,13 @@ test.describe("AttributeMap", async () => {
115115
expect(propValue).toBe("lookup-test");
116116
});
117117

118-
test("should update definition attributeLookup with dashes removed for dash-case attributes", async ({
118+
test("should update definition attributeLookup for foobar property", async ({
119119
page,
120120
}) => {
121121
const element = page.locator("attribute-map-test-element");
122122

123-
// setAttribute with dash-case triggers attributeChangedCallback for the dashes-removed property
124-
await element.evaluate(node => node.setAttribute("foo-bar", "lookup-bar-test"));
123+
// setAttribute triggers attributeChangedCallback for the foobar property
124+
await element.evaluate(node => node.setAttribute("foobar", "lookup-bar-test"));
125125
const propValue = await element.evaluate(node => (node as any).foobar);
126126

127127
expect(propValue).toBe("lookup-bar-test");

packages/fast-html/src/components/attribute-map.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,10 @@ export class AttributeMap {
8585
}
8686

8787
/**
88-
* Removes dashes from an attribute name to produce a JS property name.
88+
* Derives a JS property name from an attribute name by removing all dashes.
89+
* Template bindings should not use dashes (e.g. use {{foobar}} not {{foo-bar}}),
90+
* but HTML attributes can have dashes (e.g. foo-bar), so this ensures
91+
* a valid JS identifier is always produced.
8992
* e.g. foo-bar → foobar
9093
*/
9194
private removeDashes(str: string): string {

packages/fast-html/test/fixtures/attribute-map/attribute-map.spec.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ test.describe("AttributeMap", async () => {
2222
expect(accessors.foobar).toBeTruthy();
2323
});
2424

25-
test("should remove dashes from attribute name to form property name", async ({
25+
test("should define @attr using binding name as both attribute and property name", async ({
2626
page,
2727
}) => {
2828
const element = page.locator("attribute-map-test-element");
2929

30-
// Setting foo-bar attribute should update the foobar property
31-
await element.evaluate(node => node.setAttribute("foo-bar", "dash-test"));
30+
// Setting foobar attribute should update the foobar property
31+
await element.evaluate(node => node.setAttribute("foobar", "dash-test"));
3232
const propValue = await element.evaluate(node => (node as any).foobar);
3333

3434
expect(propValue).toBe("dash-test");
@@ -54,12 +54,12 @@ test.describe("AttributeMap", async () => {
5454
await expect(page.locator(".foo-value")).toHaveText("hello-via-attr");
5555
});
5656

57-
test("should update template when foo-bar attribute is set via setAttribute", async ({
57+
test("should update template when foobar attribute is set via setAttribute", async ({
5858
page,
5959
}) => {
6060
const element = page.locator("attribute-map-test-element");
6161

62-
await element.evaluate(node => node.setAttribute("foo-bar", "world-via-attr"));
62+
await element.evaluate(node => node.setAttribute("foobar", "world-via-attr"));
6363

6464
await expect(page.locator(".foo-bar-value")).toHaveText("world-via-attr");
6565
});
@@ -69,7 +69,7 @@ test.describe("AttributeMap", async () => {
6969

7070
await element.evaluate(node => {
7171
node.setAttribute("foo", "multi-foo");
72-
node.setAttribute("foo-bar", "multi-bar");
72+
node.setAttribute("foobar", "multi-bar");
7373
});
7474

7575
await expect(page.locator(".foo-value")).toHaveText("multi-foo");
@@ -90,7 +90,7 @@ test.describe("AttributeMap", async () => {
9090
expect(attrValue).toBe("reflected-value");
9191
});
9292

93-
test("should reflect foobar property value back to foo-bar attribute", async ({
93+
test("should reflect foobar property value back to foobar attribute", async ({
9494
page,
9595
}) => {
9696
const element = page.locator("attribute-map-test-element");
@@ -101,7 +101,7 @@ test.describe("AttributeMap", async () => {
101101

102102
await page.evaluate(() => new Promise(r => requestAnimationFrame(r)));
103103

104-
const attrValue = await element.evaluate(node => node.getAttribute("foo-bar"));
104+
const attrValue = await element.evaluate(node => node.getAttribute("foobar"));
105105
expect(attrValue).toBe("bar-reflected");
106106
});
107107

packages/fast-html/test/fixtures/attribute-map/index.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
<f-template name="attribute-map-test-element">
1111
<template>
1212
<div class="foo-value">{{foo}}</div>
13-
<div class="foo-bar-value">{{foo-bar}}</div>
13+
<div class="foo-bar-value">{{foobar}}</div>
1414
<button type="button" @click="{setFoo()}">Set Foo</button>
1515
<button type="button" @click="{setFooBar()}">Set FooBar</button>
1616
<button type="button" @click="{setMultiple()}">Set Multiple</button>

0 commit comments

Comments
 (0)