Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(shared-object-base): Move to recommended eslint base config #23233

Merged
merged 19 commits into from
Dec 3, 2024
Merged
5 changes: 1 addition & 4 deletions packages/dds/shared-object-base/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@
*/

module.exports = {
extends: [
require.resolve("@fluidframework/eslint-config-fluid/minimal-deprecated"),
"prettier",
],
extends: [require.resolve("@fluidframework/eslint-config-fluid/recommended"), "prettier"],
parserOptions: {
project: ["./tsconfig.json", "./src/test/tsconfig.json"],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@

// @alpha (undocumented)
export interface IFluidSerializer {
decode(input: any): any;
encode(value: any, bind: IFluidHandle): any;
decode(input: unknown): any;
encode(value: unknown, bind: IFluidHandle): any;
parse(value: string): any;
stringify(value: any, bind: IFluidHandle): string;
stringify(value: unknown, bind: IFluidHandle): string;
}

// @alpha
Expand All @@ -33,10 +33,10 @@ export interface ISharedObjectKind<TSharedObject> {
}

// @alpha
export function makeHandlesSerializable(value: any, serializer: IFluidSerializer, bind: IFluidHandle): any;
export function makeHandlesSerializable(value: unknown, serializer: IFluidSerializer, bind: IFluidHandle): any;
Copy link
Contributor Author

@alexvy86 alexvy86 Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating an input type on free functions shouldn't be breaking.


// @alpha
export function parseHandles(value: any, serializer: IFluidSerializer): any;
export function parseHandles(value: unknown, serializer: IFluidSerializer): any;

// @alpha
export abstract class SharedObject<TEvent extends ISharedObjectEvents = ISharedObjectEvents> extends SharedObjectCore<TEvent> {
Expand All @@ -53,7 +53,7 @@ export abstract class SharedObject<TEvent extends ISharedObjectEvents = ISharedO
// @alpha
export abstract class SharedObjectCore<TEvent extends ISharedObjectEvents = ISharedObjectEvents> extends EventEmitterWithErrorHandling<TEvent> implements ISharedObject<TEvent> {
constructor(id: string, runtime: IFluidDataStoreRuntime, attributes: IChannelAttributes);
protected abstract applyStashedOp(content: any): void;
protected abstract applyStashedOp(content: unknown): void;
// (undocumented)
readonly attributes: IChannelAttributes;
bindToContext(): void;
Expand All @@ -62,7 +62,7 @@ export abstract class SharedObjectCore<TEvent extends ISharedObjectEvents = ISha
protected get deltaManager(): IDeltaManager<ISequencedDocumentMessage, IDocumentMessage>;
protected didAttach(): void;
protected dirty(): void;
emit(event: EventEmitterEventType, ...args: any[]): boolean;
emit(event: EventEmitterEventType, ...args: unknown[]): boolean;
abstract getAttachSummary(fullTree?: boolean, trackState?: boolean, telemetryContext?: ITelemetryContext): ISummaryTreeWithStats;
abstract getGCData(fullGC?: boolean): IGarbageCollectionData;
readonly handle: IFluidHandleInternal;
Expand All @@ -79,7 +79,7 @@ export abstract class SharedObjectCore<TEvent extends ISharedObjectEvents = ISha
protected newAckBasedPromise<T>(executor: (resolve: (value: T | PromiseLike<T>) => void, reject: (reason?: any) => void) => void): Promise<T>;
protected onConnect(): void;
protected abstract onDisconnect(): any;
protected abstract processCore(message: ISequencedDocumentMessage, local: boolean, localOpMetadata: unknown): any;
protected abstract processCore(message: ISequencedDocumentMessage, local: boolean, localOpMetadata: unknown): void;
protected reSubmitCore(content: any, localOpMetadata: unknown): void;
protected rollback(content: any, localOpMetadata: unknown): void;
// (undocumented)
Expand Down
6 changes: 1 addition & 5 deletions packages/dds/shared-object-base/src/remoteObjectHandle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ import { FluidHandleBase, responseToException } from "@fluidframework/runtime-ut
* IFluidHandle can be retrieved by calling `get` on it.
*/
export class RemoteFluidObjectHandle extends FluidHandleBase<FluidObject> {
public get IFluidHandleContext() {
alexvy86 marked this conversation as resolved.
Show resolved Hide resolved
return this;
}

public readonly isAttached = true;
private objectP: Promise<FluidObject> | undefined;

Expand Down Expand Up @@ -52,7 +48,7 @@ export class RemoteFluidObjectHandle extends FluidHandleBase<FluidObject> {
};
this.objectP = this.routeContext.resolveHandle(request).then<FluidObject>((response) => {
if (response.mimeType === "fluid/object") {
const fluidObject: FluidObject = response.value;
const fluidObject: FluidObject = response.value as FluidObject;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any validation we can do here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not familiar with the space, but I think the existing check for mimeType is enough?

return fluidObject;
}
throw responseToException(response, request);
Expand Down
60 changes: 36 additions & 24 deletions packages/dds/shared-object-base/src/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,19 @@
*/

// RATIONALE: Many methods consume and return 'any' by necessity.
alexvy86 marked this conversation as resolved.
Show resolved Hide resolved
/* eslint-disable @typescript-eslint/no-unsafe-return */

import { IFluidHandle } from "@fluidframework/core-interfaces";
import {
IFluidHandleContext,
type IFluidHandleInternal,
} from "@fluidframework/core-interfaces/internal";
import { assert } from "@fluidframework/core-utils/internal";
import {
generateHandleContextPath,
isSerializedHandle,
isFluidHandle,
toFluidHandleInternal,
type ISerializedHandle,
} from "@fluidframework/runtime-utils/internal";

import { RemoteFluidObjectHandle } from "./remoteObjectHandle.js";
Expand All @@ -32,7 +33,8 @@ export interface IFluidSerializer {
* The original `input` object is not mutated. This method will shallowly clones all objects in the path from
* the root to any replaced handles. (If no handles are found, returns the original object.)
*/
encode(value: any, bind: IFluidHandle): any;
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- TODO: use unknown (breaking change)
alexvy86 marked this conversation as resolved.
Show resolved Hide resolved
encode(value: unknown, bind: IFluidHandle): any;

/**
* Given a fully-jsonable object tree that may have encoded handle objects embedded within, will return an
Expand All @@ -43,17 +45,19 @@ export interface IFluidSerializer {
*
* The decoded handles are implicitly bound to the handle context of this serializer.
*/
decode(input: any): any;
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- TODO: use unknown (breaking change)
decode(input: unknown): any;

/**
* Stringifies a given value. Converts any IFluidHandle to its stringified equivalent.
*/
stringify(value: any, bind: IFluidHandle): string;
stringify(value: unknown, bind: IFluidHandle): string;

/**
* Parses the given JSON input string and returns the JavaScript object defined by it. Any Fluid
* handles will be realized as part of the parse
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- TODO: use unknown (breaking change)
parse(value: string): any;
}

Expand All @@ -71,7 +75,7 @@ export class FluidSerializer implements IFluidSerializer {
}
}

public get IFluidSerializer() {
public get IFluidSerializer(): IFluidSerializer {
return this;
}

Expand All @@ -84,12 +88,14 @@ export class FluidSerializer implements IFluidSerializer {
*
* Any unbound handles encountered are bound to the provided IFluidHandle.
*/
public encode(input: any, bind: IFluidHandle) {
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any -- TODO: ddsFuzzHarness breaks
alexvy86 marked this conversation as resolved.
Show resolved Hide resolved
public encode(input: any, bind: IFluidHandleInternal): any {
// If the given 'input' cannot contain handles, return it immediately. Otherwise,
// return the result of 'recursivelyReplace()'.
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
return !!input && typeof input === "object"
? this.recursivelyReplace(input, this.encodeValue, bind)
? // eslint-disable-next-line @typescript-eslint/no-unsafe-argument -- TODO: ddsFuzzHarness breaks
this.recursivelyReplace(input, this.encodeValue, bind)
: input;
}

Expand All @@ -102,7 +108,8 @@ export class FluidSerializer implements IFluidSerializer {
*
* The decoded handles are implicitly bound to the handle context of this serializer.
*/
public decode(input: any) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- TODO: ddsFuzzHarness breaks
public decode(input: unknown): any {
// If the given 'input' cannot contain handles, return it immediately. Otherwise,
// return the result of 'recursivelyReplace()'.
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
Expand All @@ -111,28 +118,30 @@ export class FluidSerializer implements IFluidSerializer {
: input;
}

public stringify(input: unknown, bind: IFluidHandle) {
public stringify(input: unknown, bind: IFluidHandle): string {
const bindInternal = toFluidHandleInternal(bind);
return JSON.stringify(input, (key, value) => this.encodeValue(value, bindInternal));
}

// Parses the serialized data - context must match the context with which the JSON was stringified
public parse(input: string) {
public parse(input: string): unknown {
return JSON.parse(input, (key, value) => this.decodeValue(value));
}

// If the given 'value' is an IFluidHandle, returns the encoded IFluidHandle.
// Otherwise returns the original 'value'. Used by 'encode()' and 'stringify()'.
private readonly encodeValue = (value: unknown, bind: IFluidHandleInternal) => {
private readonly encodeValue = (value: unknown, bind?: IFluidHandleInternal): unknown => {
// If 'value' is an IFluidHandle return its encoded form.
return isFluidHandle(value)
? this.serializeHandle(toFluidHandleInternal(value), bind)
: value;
if (isFluidHandle(value)) {
assert(bind !== undefined, "Cannot encode a handle without a bind context");
return this.serializeHandle(toFluidHandleInternal(value), bind);
}
return value;
};

// If the given 'value' is an encoded IFluidHandle, returns the decoded IFluidHandle.
// Otherwise returns the original 'value'. Used by 'decode()' and 'parse()'.
private readonly decodeValue = (value: any) => {
private readonly decodeValue = (value: unknown): unknown => {
// If 'value' is a serialized IFluidHandle return the deserialized result.
if (isSerializedHandle(value)) {
// Old documents may have handles with relative path in their summaries. Convert these to absolute
Expand All @@ -150,11 +159,11 @@ export class FluidSerializer implements IFluidSerializer {
// Invoked for non-null objects to recursively replace references to IFluidHandles.
// Clones as-needed to avoid mutating the `input` object. If no IFluidHandes are present,
// returns the original `input`.
private recursivelyReplace(
input: any,
replacer: (input: any, context: any) => any,
context?: any,
) {
private recursivelyReplace<T = unknown>(
alexvy86 marked this conversation as resolved.
Show resolved Hide resolved
input: object,
replacer: (input: unknown, context?: T) => unknown,
context?: T,
): unknown {
// Note: Caller is responsible for ensuring that `input` is defined / non-null.
// (Required for Object.keys() below.)

Expand All @@ -171,7 +180,7 @@ export class FluidSerializer implements IFluidSerializer {
// Otherwise descend into the object graph looking for IFluidHandle instances.
let clone: object | undefined;
for (const key of Object.keys(input)) {
const value = input[key];
const value: unknown = input[key];
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
if (!!value && typeof value === "object") {
// Note: Except for IFluidHandle, `input` must not contain circular references (as object must
Expand All @@ -184,18 +193,21 @@ 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 });

// Overwrite the current property `key` in the clone with the `replaced` value.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
clone![key] = replaced;
clone[key] = replaced;
}
}
}
return clone ?? input;
}

protected serializeHandle(handle: IFluidHandleInternal, bind: IFluidHandleInternal) {
protected serializeHandle(
handle: IFluidHandleInternal,
bind: IFluidHandleInternal,
): ISerializedHandle {
bind.bind(handle);
return {
type: "__fluid_handle__",
Expand Down
Loading
Loading