Skip to content

Commit

Permalink
improvement(shared-object-base): use shallowCloneObject helper in Flu…
Browse files Browse the repository at this point in the history
…idSerializer to clean up `no-unsafe-assignment` lint disable (#23717)

## Description
Follow up to #23662:
#23662 (comment)

Reason for `shallowCloneObject`:
#23662 (comment)

This PR also removes several unused `no-unsafe-assignment` lint disables
in test file serializer.spec.ts.
  • Loading branch information
WillieHabi authored Jan 31, 2025
1 parent c91b39f commit 3459304
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 13 deletions.
5 changes: 2 additions & 3 deletions packages/dds/shared-object-base/src/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
IFluidHandleContext,
type IFluidHandleInternal,
} from "@fluidframework/core-interfaces/internal";
import { assert } from "@fluidframework/core-utils/internal";
import { assert, shallowCloneObject } from "@fluidframework/core-utils/internal";
import {
generateHandleContextPath,
isSerializedHandle,
Expand Down Expand Up @@ -185,8 +185,7 @@ export class FluidSerializer implements IFluidSerializer {
// current property is replaced by the `replaced` value.
if (replaced !== value) {
// Lazily create a shallow clone of the `input` object if we haven't done so already.
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- TODO: not sure if there's a good solution
clone = clone ?? (Array.isArray(input) ? [...input] : { ...input });
clone ??= shallowCloneObject(input);

// Overwrite the current property `key` in the clone with the `replaced` value.
clone[key] = replaced;
Expand Down
10 changes: 0 additions & 10 deletions packages/dds/shared-object-base/src/test/serializer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,13 @@ describe("FluidSerializer", () => {
// Verify that `encode` is a no-op for these simple cases.
for (const input of simple) {
it(`${printHandle(input)} -> ${JSON.stringify(input)}`, () => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- test deals with several object shapes
const actual = serializer.encode(input, handle);
assert.strictEqual(
actual,
input,
"encode() on input with no handles must return original input.",
);

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- test deals with several object shapes
const decoded = serializer.decode(actual);
assert.strictEqual(
decoded,
Expand Down Expand Up @@ -120,15 +118,13 @@ describe("FluidSerializer", () => {

for (const input of tricky) {
it(`${printHandle(input)} -> ${JSON.stringify(input)}`, () => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- test deals with several object shapes
const actual = serializer.encode(input, handle);
assert.strictEqual(
actual,
input,
"encode() on input with no handles must return original input.",
);

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- test deals with several object shapes
const decoded = serializer.decode(actual);
assert.strictEqual(
decoded,
Expand Down Expand Up @@ -177,7 +173,6 @@ describe("FluidSerializer", () => {

function check(decodedForm, encodedForm): void {
it(`${printHandle(decodedForm)} -> ${JSON.stringify(encodedForm)}`, () => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- TODO: update when we can fix the return type of serializer.encode()
const replaced = serializer.encode(decodedForm, handle);
assert.notStrictEqual(
replaced,
Expand All @@ -186,11 +181,9 @@ describe("FluidSerializer", () => {
);
assert.deepStrictEqual(replaced, encodedForm, "encode() must return expected output.");

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- TODO: update when we can fix the return type of serializer.encode()
const replacedTwice = serializer.encode(replaced, handle);
assert.deepStrictEqual(replacedTwice, replaced, "encode should be idempotent");

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- TODO: update when we can fix the return type of serializer.decode()
const decodedRoundTrip = serializer.decode(replaced);
assert.notStrictEqual(
decodedRoundTrip,
Expand All @@ -203,7 +196,6 @@ describe("FluidSerializer", () => {
"input must round-trip through encode()/decode().",
);

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- TODO: update when we can fix the return type of serializer.decode()
const decodedTwice = serializer.decode(decodedRoundTrip);
assert.deepStrictEqual(decodedTwice, decodedRoundTrip, "decode should be idempotent");

Expand Down Expand Up @@ -251,15 +243,13 @@ describe("FluidSerializer", () => {
input.h = handle; // eslint-disable-line @typescript-eslint/no-unsafe-member-access
input.o1.h = handle; // eslint-disable-line @typescript-eslint/no-unsafe-member-access

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- TODO: update when we can fix the return type of serializer.encode()
const replaced = serializer.encode(input, handle);
assert.notStrictEqual(
replaced,
input,
"encode() must shallow-clone rather than mutate original object.",
);

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- TODO: update when we can fix the return type of serializer.decode()
const decoded = serializer.decode(replaced);
assert.notStrictEqual(
decoded,
Expand Down

0 comments on commit 3459304

Please sign in to comment.