Skip to content

Commit 8f1e981

Browse files
tintinthongbackspace
tintinthong
and
backspace
authoredMar 10, 2025
Re-ordering linksToMany doesn't preserve templates (and UI component state) (#2241)
* add test for re-rodering linksToMany items * Fix reordering issue where finding element is based upon value that is already set. The getter was not tracking correct value * fix lint fix more lint * add box unit test * make re-order example for 3 items * fix lint * improve test naming * fix more english --------- Co-authored-by: Buck Doyle <buck.doyle@cardstack.com>
1 parent 160dab0 commit 8f1e981

File tree

3 files changed

+216
-4
lines changed

3 files changed

+216
-4
lines changed
 

‎packages/base/card-api.gts

+8-4
Original file line numberDiff line numberDiff line change
@@ -3100,6 +3100,7 @@ export class Box<T> {
31003100
}
31013101

31023102
private prevChildren: Box<ElementType<T>>[] = [];
3103+
private prevValues: ElementType<T>[] = [];
31033104

31043105
get children(): Box<ElementType<T>>[] {
31053106
if (this.state.type === 'root') {
@@ -3117,10 +3118,10 @@ export class Box<T> {
31173118
);
31183119
}
31193120

3120-
let { prevChildren, state } = this;
3121+
let { prevChildren, prevValues, state } = this;
31213122
let newChildren: Box<ElementType<T>>[] = value.map((element, index) => {
3122-
let found = prevChildren.find((oldBox, i) =>
3123-
state.useIndexBasedKeys ? index === i : oldBox.value === element,
3123+
let found = prevChildren.find((_oldBox, i) =>
3124+
state.useIndexBasedKeys ? index === i : this.prevValues[i] === element,
31243125
);
31253126
if (found) {
31263127
if (state.useIndexBasedKeys) {
@@ -3129,7 +3130,9 @@ export class Box<T> {
31293130
// mutating a watched array in a rerender will spawn another rerender which
31303131
// infinitely recurses.
31313132
} else {
3132-
prevChildren.splice(prevChildren.indexOf(found), 1);
3133+
let toRemoveIndex = prevChildren.indexOf(found);
3134+
prevChildren.splice(toRemoveIndex, 1);
3135+
prevValues.splice(toRemoveIndex, 1);
31333136
if (found.state.type === 'root') {
31343137
throw new Error('bug');
31353138
}
@@ -3146,6 +3149,7 @@ export class Box<T> {
31463149
}
31473150
});
31483151
this.prevChildren = newChildren;
3152+
this.prevValues = newChildren.map((child) => child.value);
31493153
return newChildren;
31503154
}
31513155
}

‎packages/host/tests/integration/components/card-basics-test.gts

+153
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import {
1010
triggerEvent,
1111
} from '@ember/test-helpers';
1212

13+
import { tracked } from '@glimmer/tracking';
14+
1315
import percySnapshot from '@percy/ember';
1416
import format from 'date-fns/format';
1517
import parseISO from 'date-fns/parseISO';
@@ -2283,6 +2285,157 @@ module('Integration | card-basics', function (hooks) {
22832285
await waitUntil(() => cleanWhiteSpace(root.textContent!) === 'Quint');
22842286
});
22852287

2288+
test('Re-ordering items in a linksToMany field will preserve the template and box component state', async function (assert) {
2289+
class Fitted extends Component<typeof Pet1> {
2290+
@tracked counter = 0;
2291+
2292+
incrementCounter = () => {
2293+
this.counter++;
2294+
};
2295+
<template>
2296+
{{@model.name}}
2297+
<button
2298+
{{on 'click' this.incrementCounter}}
2299+
data-test-increment-counter
2300+
>Increment</button>
2301+
<div data-test-counter>
2302+
{{this.counter}}
2303+
</div>
2304+
</template>
2305+
}
2306+
2307+
class FittedPrime extends Component<typeof Pet1Prime> {
2308+
@tracked counter = 0;
2309+
2310+
incrementCounter = () => {
2311+
this.counter++;
2312+
};
2313+
<template>
2314+
<div data-test-different-template>Different Template</div>
2315+
{{@model.name}}
2316+
<button
2317+
{{on 'click' this.incrementCounter}}
2318+
data-test-increment-counter
2319+
>Increment</button>
2320+
<div data-test-counter>
2321+
{{this.counter}}
2322+
</div>
2323+
</template>
2324+
}
2325+
class Pet1 extends CardDef {
2326+
@field name = contains(StringField);
2327+
static fitted = Fitted;
2328+
}
2329+
2330+
class Pet1Prime extends Pet1 {
2331+
static fitted = FittedPrime;
2332+
}
2333+
2334+
class Person1 extends CardDef {
2335+
@field pets = linksToMany(Pet1);
2336+
static isolated = class Embedded extends Component<typeof this> {
2337+
reorder = () => {
2338+
if (
2339+
this.args.model.pets &&
2340+
this.args.model.pets[0] &&
2341+
this.args.model.pets[1]
2342+
) {
2343+
this.args.model.pets = [
2344+
this.args.model.pets[1],
2345+
this.args.model.pets[0],
2346+
];
2347+
}
2348+
};
2349+
<template>
2350+
<button
2351+
{{on 'click' this.reorder}}
2352+
data-test-reorder
2353+
>Reorder</button>
2354+
<@fields.pets @format='fitted' />
2355+
</template>
2356+
};
2357+
}
2358+
2359+
loader.shimModule(`${testRealmURL}test-cards`, {
2360+
Pet1,
2361+
Pet1Prime,
2362+
Person1,
2363+
});
2364+
2365+
let pet1 = new Pet1({ name: 'jersey' });
2366+
let pet2 = new Pet1Prime({ name: 'boboy' });
2367+
await saveCard(pet1, `${testRealmURL}Pet/pet1`, loader);
2368+
await saveCard(pet2, `${testRealmURL}Pet/pet2`, loader);
2369+
let person = new Person1({
2370+
name: 'Mango',
2371+
pets: [pet1, pet2],
2372+
});
2373+
await renderCard(loader, person, 'isolated');
2374+
assert
2375+
.dom(`[data-test-plural-view-item="0"] [data-test-counter]`)
2376+
.hasText('0');
2377+
assert
2378+
.dom(
2379+
`[data-test-plural-view-item="0"][data-test-card="${testRealmURL}Pet/pet1"]`,
2380+
)
2381+
.containsText('jersey');
2382+
2383+
await click(
2384+
`[data-test-plural-view-item="0"] [data-test-increment-counter]`,
2385+
);
2386+
await click(
2387+
`[data-test-plural-view-item="0"] [data-test-increment-counter]`,
2388+
);
2389+
assert
2390+
.dom(`[data-test-plural-view-item="0"] [data-test-counter]`)
2391+
.hasText('2');
2392+
assert
2393+
.dom(
2394+
`[data-test-plural-view-item="0"][data-test-card="${testRealmURL}Pet/pet1"]`,
2395+
)
2396+
.containsText('jersey');
2397+
assert
2398+
.dom(`[data-test-plural-view-item="0"] [data-test-different-template]`)
2399+
.doesNotExist();
2400+
assert
2401+
.dom(`[data-test-plural-view-item="1"] [data-test-counter]`)
2402+
.hasText('0');
2403+
assert
2404+
.dom(
2405+
`[data-test-plural-view-item="1"][data-test-card="${testRealmURL}Pet/pet2"]`,
2406+
)
2407+
.containsText('boboy');
2408+
assert
2409+
.dom(`[data-test-plural-view-item="1"] [data-test-different-template]`)
2410+
.exists();
2411+
await click('[data-test-reorder]'); //Reorder
2412+
assert
2413+
.dom(`[data-test-plural-view-item="0"] [data-test-counter]`)
2414+
.hasText('0');
2415+
assert
2416+
.dom(
2417+
`[data-test-plural-view-item="0"][data-test-card="${testRealmURL}Pet/pet2"]`,
2418+
)
2419+
.containsText('boboy');
2420+
assert
2421+
.dom(`[data-test-plural-view-item="0"] [data-test-counter]`)
2422+
.hasText('0');
2423+
assert
2424+
.dom(`[data-test-plural-view-item="0"] [data-test-different-template]`)
2425+
.exists();
2426+
assert
2427+
.dom(
2428+
`[data-test-plural-view-item="1"][data-test-card="${testRealmURL}Pet/pet1"]`,
2429+
)
2430+
.containsText('jersey');
2431+
assert
2432+
.dom(`[data-test-plural-view-item="1"] [data-test-counter]`)
2433+
.hasText('2');
2434+
assert
2435+
.dom(`[data-test-plural-view-item="1"] [data-test-different-template]`)
2436+
.doesNotExist();
2437+
});
2438+
22862439
test('rerender when a containsMany field is fully replaced', async function (assert) {
22872440
class Person extends CardDef {
22882441
@field pets = containsMany(StringField);

‎packages/host/tests/unit/box-test.ts

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { RenderingTestContext } from '@ember/test-helpers';
2+
3+
import { module, test } from 'qunit';
4+
5+
import { Loader, baseRealm } from '@cardstack/runtime-common';
6+
7+
import { lookupLoaderService } from '../helpers';
8+
import { setupRenderingTest } from '../helpers/setup';
9+
10+
let cardApi: typeof import('https://cardstack.com/base/card-api');
11+
12+
let loader: Loader;
13+
14+
module('Unit | box', function (hooks) {
15+
setupRenderingTest(hooks);
16+
hooks.beforeEach(function (this: RenderingTestContext) {
17+
loader = lookupLoaderService().loader;
18+
});
19+
hooks.beforeEach(async function () {
20+
cardApi = await loader.import(`${baseRealm.url}card-api`);
21+
});
22+
23+
test('Box children maintain object strict equality after re-ordering', async function (assert) {
24+
let { Box } = cardApi;
25+
let parentCardModel = {
26+
someField: [],
27+
};
28+
let childCard1Model = {};
29+
let childCard2Model = {};
30+
let childCard3Model = {};
31+
let box = Box.create(parentCardModel);
32+
let boxArr = box.field('someField');
33+
let childValues: any = [childCard1Model, childCard2Model, childCard3Model];
34+
boxArr.set(childValues);
35+
assert.strictEqual(boxArr.children[0].value, childCard1Model);
36+
assert.strictEqual(boxArr.children[1].value, childCard2Model);
37+
assert.strictEqual(boxArr.children[2].value, childCard3Model);
38+
assert.strictEqual(boxArr.children[0].name, '0');
39+
assert.strictEqual(boxArr.children[1].name, '1');
40+
assert.strictEqual(boxArr.children[2].name, '2');
41+
42+
let childValuesReordered: any = [
43+
childCard2Model,
44+
childCard3Model,
45+
childCard1Model,
46+
];
47+
boxArr.set(childValuesReordered);
48+
assert.strictEqual(boxArr.children[0].value, childCard2Model);
49+
assert.strictEqual(boxArr.children[1].value, childCard3Model);
50+
assert.strictEqual(boxArr.children[2].value, childCard1Model);
51+
assert.strictEqual(boxArr.children[0].name, '0');
52+
assert.strictEqual(boxArr.children[1].name, '1');
53+
assert.strictEqual(boxArr.children[2].name, '2');
54+
});
55+
});

0 commit comments

Comments
 (0)