Skip to content

Commit

Permalink
FluidObject: Add Breaking note, and update remaining non-synthesize c…
Browse files Browse the repository at this point in the history
…ases (#8354)

Adds a note to breaking about the deprecation of IFluidObject and updates remaining internal usages which are not synthesize which will come with #8074

fixes #8076
  • Loading branch information
anthony-murphy authored Nov 19, 2021
1 parent f9b4d1c commit 4ff22fa
Show file tree
Hide file tree
Showing 16 changed files with 60 additions and 26 deletions.
24 changes: 24 additions & 0 deletions BREAKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ There are a few steps you can take to write a good change note and avoid needing
- [close() removed from IDocumentDeltaConnection](#close-removed-from-IDocumentDeltaConnection)
- [Remove `IOdspResolvedUrl.sharingLinkToRedeem` and use `IOdspResolvedUrl.shareLinkInfo` instead](#Remove-IOdspResolvedUrl.sharingLinkToRedeem-and-use-IOdspResolvedUrl.shareLinkInfo-instead)
- [Replace `createCreateNewRequest` function with `createOdspCreateContainerRequest` function](#Replace-createCreateNewRequest-function-with-createOdspCreateContainerRequest-function)
- [Deprecate IFluidObject and introduce FluidObject](#Deprecate-IFluidObject-and-introduce-FluidObject)

### `chaincodePackage` removed from `Container`
The `chaincodePackage` property on `Container` was deprecated in 0.28, and has now been removed. Two new APIs have been added to replace its functionality, `getSpecifiedCodeDetails()` and `getLoadedCodeDetails()`. Use `getSpecifiedCodeDetails()` to get the code details currently specified for the `Container`, or `getLoadedCodeDetails()` to get the code details that were used to load the `Container`.
Expand All @@ -29,6 +30,29 @@ The `sharingLinkToRedeem` property is removed from the `IOdspResolvedUrl` interf
### Replace `createCreateNewRequest` function with `createOdspCreateContainerRequest` function
The `createCreateNewRequest()` is removed and replaced with `createOdspCreateContainerRequest()` in the `odsp-driver` package. If any instances of `createCreateNewRequest()` are used, replace them with `createOdspCreateContainerRequest()` by importing it from `@fluidframework/odsp-driver` package.

### Deprecate IFluidObject and introduce FluidObject
This release deprecates the interface `IFluidObject` and introduces the utility type [`FluidObject`](https://github.com/microsoft/FluidFramework/blob/main/common/lib/core-interfaces/src/provider.ts). The primary reason for this change is that the module augmentation used by `IFluidObject` creates excessive type coupling where a small breaking change in any type exposed off `IFluidObject` can lead to type error in all usages of `IFluidObject`.
On investigation we also found that the uber type `IFluidObject` wasn't genenerally necessary, as consumers generally only used a small number of specific types that they knew in advance.

Given these points, we've introduced [`FluidObject`](https://github.com/microsoft/FluidFramework/blob/main/common/lib/core-interfaces/src/provider.ts). `FluidObject` is a utility type that is used in both its generic and non-generic forms.

The non-generic `FluidObject` is returned or taken in cases where the specific functionally isn't known, or is different based on scenario. You'll see this usage for things like `scope` and the request pattern.

The non-generic `FluidObject` is a hint that the generic form of `FluidObject` should be used to inspect it. For example
``` typescript
const provider: FluidObject<IFluidHTMLView> = requestFluidObject(container, "/");
if(provider.IFluidHTMLView !== undefined){
provider.IFluidHTMLView.render(div)
}
```

If you want to inspect for multiple interfaces via `FluidObject`, you can use an intersection:
``` typescript
const provider: FluidObject<IFluidHTMLView & IFluidMountableView> = requestFluidObject(container, "/");
```

Please begin reducing the usage of `IFluidObject` and moving to `FluidObject`. If you find any cases that `FluidObject` doesn't support please file an issue.

## 0.51 Breaking changes
- [`maxMessageSize` property has been deprecated from IConnectionDetails and IDocumentDeltaConnection](#maxmessagesize-property-has-been-deprecated-from-iconnectiondetails-and-idocumentdeltaconnection)
- [_createDataStoreWithProps and IFluidDataStoreChannel](#createdatastorewithprops-and-ifluiddatastorechannel)
Expand Down
2 changes: 1 addition & 1 deletion api-report/aqueduct.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export abstract class PureDataObject<O extends IFluidObject = object, S = undefi
// (undocumented)
static getDataObject(runtime: IFluidDataStoreRuntime): Promise<PureDataObject<object, undefined, IEvent>>;
getFluidObjectFromDirectory<T extends IFluidObject & FluidObject & IFluidLoadable>(key: string, directory: IDirectory, getObjectFromDirectory?: (id: string, directory: IDirectory) => string | IFluidHandle | undefined): Promise<T | undefined>;
protected getService<T extends IFluidObject>(id: string): Promise<T>;
protected getService<T extends IFluidObject & FluidObject>(id: string): Promise<T>;
get handle(): IFluidHandle<this>;
protected hasInitialized(): Promise<void>;
// (undocumented)
Expand Down
3 changes: 2 additions & 1 deletion api-report/datastore.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { AttachState } from '@fluidframework/container-definitions';
import { ContainerWarning } from '@fluidframework/container-definitions';
import { FluidObject } from '@fluidframework/core-interfaces';
import { FluidSerializer } from '@fluidframework/runtime-utils';
import { IAudience } from '@fluidframework/container-definitions';
import { IChannel } from '@fluidframework/datastore-definitions';
Expand Down Expand Up @@ -125,7 +126,7 @@ export class FluidDataStoreRuntime extends TypedEventEmitter<IFluidDataStoreRunt
}

// @public (undocumented)
export class FluidObjectHandle<T extends IFluidObject = IFluidObject> implements IFluidHandle {
export class FluidObjectHandle<T extends FluidObject = IFluidObject> implements IFluidHandle {
constructor(value: T, path: string, routeContext: IFluidHandleContext);
// (undocumented)
readonly absolutePath: string;
Expand Down
3 changes: 2 additions & 1 deletion api-report/runtime-utils.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
```ts

import { FluidObject } from '@fluidframework/core-interfaces';
import { IChannelStorageService } from '@fluidframework/datastore-definitions';
import { IContainerContext } from '@fluidframework/container-definitions';
import { IContainerRuntime } from '@fluidframework/container-runtime-definitions';
Expand Down Expand Up @@ -166,7 +167,7 @@ export type RefreshSummaryResult = {
};

// @public (undocumented)
export function requestFluidObject<T = IFluidObject>(router: IFluidRouter, url: string | IRequest): Promise<T>;
export function requestFluidObject<T = IFluidObject & FluidObject>(router: IFluidRouter, url: string | IRequest): Promise<T>;

// @public
export class RequestParser implements IRequest {
Expand Down
3 changes: 2 additions & 1 deletion api-report/sequence.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { BaseSegment } from '@fluidframework/merge-tree';
import { Client } from '@fluidframework/merge-tree';
import { Deferred } from '@fluidframework/common-utils';
import { FluidObject } from '@fluidframework/core-interfaces';
import { IChannelAttributes } from '@fluidframework/datastore-definitions';
import { IChannelFactory } from '@fluidframework/datastore-definitions';
import { IChannelServices } from '@fluidframework/datastore-definitions';
Expand Down Expand Up @@ -663,7 +664,7 @@ export class SparseMatrix extends SharedSegmentSequence<MatrixSegment> {
static create(runtime: IFluidDataStoreRuntime, id?: string): SparseMatrix;
static getFactory(): IChannelFactory;
// (undocumented)
getItem(row: number, col: number): Jsonable<string | number | boolean | IFluidHandle<IFluidObject & IFluidLoadable>>;
getItem(row: number, col: number): Jsonable<string | number | boolean | IFluidHandle<IFluidObject & FluidObject & IFluidLoadable>>;
// (undocumented)
getPositionProperties(row: number, col: number): PropertySet;
// (undocumented)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export interface IFluidConfiguration extends IProvideFluidConfiguration {
export const IFluidHandle: keyof IProvideFluidHandle;

// @public
export interface IFluidHandle<T = IFluidObject & IFluidLoadable> extends IProvideFluidHandle {
export interface IFluidHandle<T = IFluidObject & FluidObject & IFluidLoadable> extends IProvideFluidHandle {
// @deprecated (undocumented)
readonly absolutePath: string;
attachGraph(): void;
Expand Down
5 changes: 3 additions & 2 deletions common/lib/core-interfaces/src/handles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { IRequest, IResponse } from "./fluidRouter";
import { IFluidObject } from "./fluidObject";
import { IFluidLoadable } from "./fluidLoadable";
import { FluidObject } from "./provider";

export const IFluidHandleContext: keyof IProvideFluidHandleContext = "IFluidHandleContext";

Expand Down Expand Up @@ -51,8 +52,8 @@ export interface IProvideFluidHandle {
* Handle to a shared FluidObject
*/
export interface IFluidHandle<
// REVIEW: Constrain `T` to `IFluidObject & IFluidLoadable`?
T = IFluidObject & IFluidLoadable
// REVIEW: Constrain `T` to something? How do we support dds and datastores safely?
T = IFluidObject & FluidObject & IFluidLoadable
> extends IProvideFluidHandle {

/**
Expand Down
2 changes: 1 addition & 1 deletion examples/data-objects/prosemirror/src/componentView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export class ComponentView implements NodeView {
throw new Error("Can't insert a non-fluid component");
}

const component = result.value as FluidObject;
const component: FluidObject = result.value;
if (!HTMLViewAdapter.canAdapt(component)) {
throw new Error("Don't know how to render this component");
}
Expand Down
4 changes: 2 additions & 2 deletions packages/dds/sequence/src/sparsematrix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License.
*/

import { IFluidHandle, IFluidLoadable, IFluidObject } from "@fluidframework/core-interfaces";
import { FluidObject, IFluidHandle, IFluidLoadable, IFluidObject } from "@fluidframework/core-interfaces";
import {
BaseSegment,
createGroupOp,
Expand Down Expand Up @@ -277,7 +277,7 @@ export class SparseMatrix extends SharedSegmentSequence<MatrixSegment> {
public getItem(row: number, col: number):
// The return type is defined explicitly here to prevent TypeScript from generating dynamic imports
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-arguments
Jsonable<string | number | boolean | IFluidHandle<IFluidObject & IFluidLoadable>> {
Jsonable<string | number | boolean | IFluidHandle<IFluidObject & FluidObject & IFluidLoadable>> {
const pos = rowColToPosition(row, col);
const { segment, offset } =
this.getContainingSegment(pos);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ export abstract class PureDataObject<O extends IFluidObject = object, S = undefi
* Gets the service at a given id.
* @param id - service id
*/
protected async getService<T extends IFluidObject>(id: string): Promise<T> {
protected async getService<T extends IFluidObject & FluidObject>(id: string): Promise<T> {
return handleFromLegacyUri<T>(`/${serviceRoutePathRoot}/${id}`, this.context.containerRuntime).get();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import {
IRuntimeFactory,
} from "@fluidframework/container-definitions";
import {
FluidObject,
IFluidCodeDetails,
IFluidObject,
} from "@fluidframework/core-interfaces";
import {
IQuorum,
Expand Down Expand Up @@ -62,7 +62,7 @@ describe("ContainerContext Tests", () => {
) => {
return ContainerContext.createOrLoad(
(mockContainer as unknown) as Container,
(sandbox.stub() as unknown) as IFluidObject,
(sandbox.stub() as unknown) as FluidObject,
codeLoader as ICodeDetailsLoader,
quorumCodeDetails,
undefined,
Expand Down
5 changes: 3 additions & 2 deletions packages/runtime/datastore/src/fluidHandle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import {
IFluidObject,
IFluidHandle,
IFluidHandleContext,
FluidObject,
} from "@fluidframework/core-interfaces";
import { AttachState } from "@fluidframework/container-definitions";
import { generateHandleContextPath } from "@fluidframework/runtime-utils";

export class FluidObjectHandle<T extends IFluidObject = IFluidObject> implements IFluidHandle {
export class FluidObjectHandle<T extends FluidObject = IFluidObject> implements IFluidHandle {
// This is used to break the recursion while attaching the graph. Also tells the attach state of the graph.
private graphAttachState: AttachState = AttachState.Detached;
private bound: Set<IFluidHandle> | undefined;
Expand All @@ -25,7 +26,7 @@ export class FluidObjectHandle<T extends IFluidObject = IFluidObject> implements

/**
* Creates a new FluidObjectHandle.
* @param value - The IFluidObject object this handle is for.
* @param value - The FluidObject object this handle is for.
* @param path - The path to this handle relative to the routeContext.
* @param routeContext - The parent IFluidHandleContext that has a route to this handle.
*/
Expand Down
6 changes: 4 additions & 2 deletions packages/runtime/runtime-definitions/src/dataStoreRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
import { IProvideFluidDataStoreFactory } from "./dataStoreFactory";

declare module "@fluidframework/core-interfaces" {
// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface IFluidObject extends Readonly<Partial<IProvideFluidDataStoreRegistry>> { }
export interface IFluidObject {
/** @deprecated - use `FluidObject<IFluidDataStoreRegistry>` instead */
readonly IFluidDataStoreRegistry?: IFluidDataStoreRegistry
}
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/runtime/runtime-utils/src/dataStoreHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import { assert } from "@fluidframework/common-utils";
import {
FluidObject,
IFluidObject,
IFluidRouter,
IRequest,
Expand Down Expand Up @@ -71,7 +72,7 @@ export function responseToException(response: IResponse, request: IRequest) {
return err;
}

export async function requestFluidObject<T = IFluidObject>(
export async function requestFluidObject<T = IFluidObject & FluidObject>(
router: IFluidRouter, url: string | IRequest): Promise<T> {
const request = typeof url === "string" ? { url } : url;
const response = await router.request(request);
Expand Down
12 changes: 7 additions & 5 deletions packages/runtime/runtime-utils/src/remoteObjectHandle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@

import { assert } from "@fluidframework/common-utils";
import {
IFluidObject,
IFluidHandle,
IFluidHandleContext,
IRequest,
IResponse,
FluidObject,
IFluidRouter,
} from "@fluidframework/core-interfaces";
import { create404Response, exceptionToResponse, responseToException } from "./dataStoreHelpers";

Expand All @@ -26,7 +27,7 @@ export class RemoteFluidObjectHandle implements IFluidHandle {
public get IFluidHandle() { return this; }

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

/**
* Creates a new RemoteFluidObjectHandle when parsing an IFluidHandle.
Expand All @@ -51,9 +52,10 @@ export class RemoteFluidObjectHandle implements IFluidHandle {
if (this.objectP === undefined) {
const request = { url: this.absolutePath };
this.objectP = this.routeContext.resolveHandle(request)
.then<IFluidObject>((response) => {
.then<FluidObject>((response) => {
if (response.mimeType === "fluid/object") {
return response.value as IFluidObject;
const fluidObject: FluidObject = response.value;
return fluidObject;
}
throw responseToException(response, request);
});
Expand All @@ -71,7 +73,7 @@ export class RemoteFluidObjectHandle implements IFluidHandle {

public async request(request: IRequest): Promise<IResponse> {
try {
const object = await this.get() as IFluidObject;
const object: FluidObject<IFluidRouter> = await this.get();
const router = object.IFluidRouter;

return router !== undefined
Expand Down
6 changes: 3 additions & 3 deletions packages/tools/webpack-fluid-loader/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
resolveFluidPackageEnvironment,
WebCodeLoader,
} from "@fluidframework/web-code-loader";
import { IFluidObject, IFluidPackage, IFluidCodeDetails } from "@fluidframework/core-interfaces";
import { IFluidPackage, IFluidCodeDetails, FluidObject } from "@fluidframework/core-interfaces";
import { IDocumentServiceFactory, IResolvedUrl } from "@fluidframework/driver-definitions";
import { LocalDocumentServiceFactory, LocalResolver } from "@fluidframework/local-driver";
import { RequestParser, createDataStoreFactory } from "@fluidframework/runtime-utils";
Expand Down Expand Up @@ -346,7 +346,7 @@ async function getFluidObjectAndRender(container: Container, url: string, div: H
return false;
}

const fluidObject = response.value as IFluidObject;
const fluidObject: FluidObject<IFluidMountableView> = response.value;
if (fluidObject === undefined) {
return;
}
Expand Down Expand Up @@ -394,7 +394,7 @@ async function attachContainer(
// generated by the backend and encoded in the resolved URL,
// as opposed to the ID requested on the client prior to attaching the container.
// NOTE: in case of an odsp container, the ID in the resolved URL cannot be used for
// referring/opening the attached conainer.
// referring/opening the attached container.
ensureFluidResolvedUrl(resolvedUrl);
docUrl = url.replace(documentId, resolvedUrl.id);
title = resolvedUrl.id;
Expand Down

0 comments on commit 4ff22fa

Please sign in to comment.