Skip to content

Commit

Permalink
Update the ArrayObserver to better account for sort and reverse (#7062)
Browse files Browse the repository at this point in the history
# Pull Request

## 📖 Description

This change updates the `ArrayObserver` to use some array tracking when applying `sort` or `reverse`. This will retain state to overcome the problem seen in #6919. No updates are required by developers.

### 🎫 Issues

Closes #6919

## ✅ Checklist

### General

<!--- Review the list and put an x in the boxes that apply. -->

- [x] I have included a change request file using `$ npm run change`
- [x] I have added tests for my changes.
- [x] I have tested my changes.
- [x] I have updated the project documentation to reflect my changes.
- [x] I have read the [CONTRIBUTING](https://github.com/microsoft/fast/blob/master/CONTRIBUTING.md) documentation and followed the [standards](https://github.com/microsoft/fast/blob/master/CODE_OF_CONDUCT.md#our-standards) for this project.
  • Loading branch information
janechu authored Jan 27, 2025
1 parent 995724f commit c4836e5
Show file tree
Hide file tree
Showing 8 changed files with 325 additions and 56 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Update the ArrayObserver to better account for sort and reverse",
"packageName": "@microsoft/fast-element",
"email": "[email protected]",
"dependentChangeType": "none"
}
20 changes: 19 additions & 1 deletion packages/web-components/fast-element/docs/api-report.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,18 @@ export type AddViewBehaviorFactory = (factory: ViewBehaviorFactory) => string;

// @public
export interface ArrayObserver extends SubscriberSet {
addSort(sort: Sort): void;
addSplice(splice: Splice): void;
flush(): void;
readonly lengthObserver: LengthObserver;
reset(oldCollection: any[] | undefined): void;
readonly sortObserver: SortObserver;
strategy: SpliceStrategy | null;
}

// @public
export const ArrayObserver: Readonly<{
readonly sorted: 0;
readonly enable: () => void;
}>;

Expand Down Expand Up @@ -774,7 +777,7 @@ export function repeat<TSource = any, TArray extends ReadonlyArray<any> = Readon
export class RepeatBehavior<TSource = any> implements ViewBehavior, Subscriber {
constructor(directive: RepeatDirective);
bind(controller: ViewController): void;
handleChange(source: any, args: Splice[] | ExpressionObserver): void;
handleChange(source: any, args: Splice[] | Sort[] | ExpressionObserver): void;
unbind(): void;
// @internal (undocumented)
views: SyntheticView[];
Expand Down Expand Up @@ -822,6 +825,21 @@ export class SlottedDirective extends NodeObservationDirective<SlottedDirectiveO
export interface SlottedDirectiveOptions<T = any> extends NodeBehaviorOptions<T>, AssignedNodesOptions {
}

// @public
export class Sort {
constructor(sorted?: number[] | undefined);
// (undocumented)
sorted?: number[] | undefined;
}

// @public
export function sortedCount<T>(array: readonly T[]): number;

// @public
export interface SortObserver extends Subscriber {
sorted: number;
}

// @public
export const SourceLifetime: Readonly<{
readonly unknown: undefined;
Expand Down
2 changes: 1 addition & 1 deletion packages/web-components/fast-element/src/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const FAST: FASTGlobal = globalThis.FAST;

const debugMessages = {
[1101 /* needsArrayObservation */]:
"Must call enableArrayObservation before observing arrays.",
"Must call ArrayObserver.enable() before observing arrays.",
[1201 /* onlySetDOMPolicyOnce */]: "The DOM Policy can only be set once.",
[1202 /* bindingInnerHTMLRequiresTrustedTypes */]:
"To bind innerHTML, you must use a TrustedTypesPolicy.",
Expand Down
3 changes: 3 additions & 0 deletions packages/web-components/fast-element/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ export {
Splice,
SpliceStrategy,
SpliceStrategySupport,
Sort,
SortObserver,
sortedCount,
} from "./observation/arrays.js";
export { UpdateQueue, Updates } from "./observation/update-queue.js";

Expand Down
76 changes: 41 additions & 35 deletions packages/web-components/fast-element/src/observation/arrays.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from "chai";
import { Observable } from "./observable.js";
import { ArrayObserver, lengthOf, Splice } from "./arrays.js";
import { ArrayObserver, lengthOf, Splice, Sort } from "./arrays.js";
import { SubscriberSet } from "./notifier.js";
import { Updates } from "./update-queue.js";

Expand Down Expand Up @@ -140,13 +140,13 @@ describe("The ArrayObserver", () => {
const array = [1, 2, 3, 4];
array.reverse();

expect(array).members([4, 3, 2, 1]);
expect(array).ordered.members([4, 3, 2, 1]);

Array.prototype.reverse.call(array);
expect(array).members([1, 2, 3, 4]);
expect(array).ordered.members([1, 2, 3, 4]);

const observer = Observable.getNotifier<ArrayObserver>(array);
let changeArgs: Splice[] | null = null;
let changeArgs: Sort[] | null = null;

observer.subscribe({
handleChange(array, args) {
Expand All @@ -155,23 +155,34 @@ describe("The ArrayObserver", () => {
});

array.reverse();
expect(array).members([4, 3, 2, 1]);
expect(array).ordered.members([4, 3, 2, 1]);

await Updates.next();

expect(changeArgs).length(1);
expect(changeArgs![0].addedCount).equal(0);
expect(changeArgs![0].removed).members([]);
expect(changeArgs![0].index).equal(0);
expect(changeArgs![0].reset).equal(true);
expect(changeArgs![0].sorted).to.have.ordered.members(
[
3,
2,
1,
0
]
);
changeArgs = null;
array.reverse();
expect(array).ordered.members([1, 2, 3, 4]);

await Updates.next();

Array.prototype.reverse.call(array);
expect(array).members([1, 2, 3, 4]);
expect(changeArgs).length(1);
expect(changeArgs![0].addedCount).equal(0);
expect(changeArgs![0].removed).members([]);
expect(changeArgs![0].index).equal(0);
expect(changeArgs![0].reset).equal(true);
expect(changeArgs![0].sorted).to.have.ordered.members(
[
3,
2,
1,
0
]
);
});

it("observes shifts", async () => {
Expand Down Expand Up @@ -216,16 +227,17 @@ describe("The ArrayObserver", () => {

it("observes sorts", async () => {
ArrayObserver.enable();
let array = [1, 2, 3, 4];
let array = [1, 3, 2, 4, 3];

array.sort((a, b) => b - a);
expect(array).members([4, 3, 2, 1]);
expect(array).ordered.members([4, 3, 3, 2, 1]);

Array.prototype.sort.call(array, (a, b) => a - b);
expect(array).members([1, 2, 3, 4]);
expect(array).ordered.members([1, 2, 3, 3, 4]);

array = [1, 3, 2, 4, 3];
const observer = Observable.getNotifier<ArrayObserver>(array);
let changeArgs: Splice[] | null = null;
let changeArgs: Sort[] | null = null;

observer.subscribe({
handleChange(array, args) {
Expand All @@ -234,26 +246,20 @@ describe("The ArrayObserver", () => {
});

array.sort((a, b) => b - a);
expect(array).members([4, 3, 2, 1]);
expect(array).ordered.members([4, 3, 3, 2, 1]);

await Updates.next();

expect(changeArgs).length(1);
expect(changeArgs![0].addedCount).equal(0);
expect(changeArgs![0].removed).members([]);
expect(changeArgs![0].index).equal(0);
expect(changeArgs![0].reset).equal(true);

Array.prototype.sort.call(array, (a, b) => a - b);
expect(array).members([1, 2, 3, 4]);

await Updates.next();

expect(changeArgs).length(1);
expect(changeArgs![0].addedCount).equal(0);
expect(changeArgs![0].removed).members([]);
expect(changeArgs![0].index).equal(0);
expect(changeArgs![0].reset).equal(true);
expect(changeArgs![0].sorted).to.have.ordered.members(
[
3,
1,
4,
2,
0
]
);
});

it("observes splices", async () => {
Expand Down
Loading

0 comments on commit c4836e5

Please sign in to comment.