From 584b7b69f4d62f7e867926d95acf0f017c7e4aeb Mon Sep 17 00:00:00 2001 From: Alex Villarreal <716334+alexvy86@users.noreply.github.com> Date: Mon, 1 May 2023 12:51:52 -0500 Subject: [PATCH] [v2int/4.1] Revert to shallow clone to reduce bundle size (#15364) --- .../loader/container-loader/src/container.ts | 9 +- .../src/test/container.spec.ts | 101 ------------------ 2 files changed, 6 insertions(+), 104 deletions(-) diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index f9f55617b124..057c93f7f006 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -5,8 +5,6 @@ // eslint-disable-next-line import/no-internal-modules import merge from "lodash/merge"; -// eslint-disable-next-line import/no-internal-modules -import cloneDeep from "lodash/cloneDeep"; import { v4 as uuid } from "uuid"; import { @@ -748,7 +746,12 @@ export class Container // Prefix all events in this file with container-loader this.mc = loggerToMonitoringContext(ChildLogger.create(this.subLogger, "Container")); - this.options = cloneDeep(this.loader.services.options); + // Warning: this is only a shallow clone. Mutation of any individual loader option will mutate it for + // all clients that were loaded from the same loader (including summarizer clients). + // Tracking alternative ways to handle this in AB#4129. + this.options = { + ...this.loader.services.options, + }; this._deltaManager = this.createDeltaManager(); diff --git a/packages/test/test-end-to-end-tests/src/test/container.spec.ts b/packages/test/test-end-to-end-tests/src/test/container.spec.ts index 425ec241f167..b2f56a145c5e 100644 --- a/packages/test/test-end-to-end-tests/src/test/container.spec.ts +++ b/packages/test/test-end-to-end-tests/src/test/container.spec.ts @@ -4,7 +4,6 @@ */ import { strict as assert } from "assert"; -import { v4 as uuid } from "uuid"; import { MockDocumentDeltaConnection } from "@fluid-internal/test-loader-utils"; import { IRequest } from "@fluidframework/core-interfaces"; import { @@ -12,7 +11,6 @@ import { ContainerErrorType, IFluidCodeDetails, IContainer, - LoaderHeader, } from "@fluidframework/container-definitions"; import { ConnectionState, @@ -50,7 +48,6 @@ import { import { IContainerRuntimeBase } from "@fluidframework/runtime-definitions"; import { ConfigTypes, IConfigProviderBase } from "@fluidframework/telemetry-utils"; import { ContainerRuntime } from "@fluidframework/container-runtime"; -import { IClient } from "@fluidframework/protocol-definitions"; const id = "fluid-test://localhost/containerTest"; const testRequest: IRequest = { url: id }; @@ -669,104 +666,6 @@ describeNoCompat("Container", (getTestObjectProvider) => { assert.strictEqual(deltaManagerClosed, 1, "DeltaManager should send closed event"); assert.strictEqual(runtimeDispose, 1, "ContainerRuntime should send dispose event"); }); - - it("clientDetailsOverride does not cause client details of other containers with the same loader to change", async function () { - const documentId = uuid(); - const client: IClient = { - details: { - capabilities: { interactive: true }, - }, - permission: [], - scopes: [], - user: { id: "" }, - mode: "write", - }; - const loaderProps: Partial = { - options: { - client, - }, - }; - const loader = provider.makeTestLoader({ loaderProps }); - const container1 = await loader.createDetachedContainer(provider.defaultCodeDetails); - const createNewRequest = provider.driver.createCreateNewRequest(documentId); - await container1.attach(createNewRequest); - - // Check that client details are the expected ones before resolving a second container with different client details - assert.equal( - (container1 as any).clientDetails?.capabilities?.interactive, - true, - "First container's client capabilities should say 'interactive: true' before resolving second container", - ); - assert.equal( - (container1 as any).clientDetails?.type, - undefined, - "First container's clientDetails should have undefined 'type' before resolving second container", - ); - - // Check that the IClient object passed in loader props hasn't been mutated - assert.equal( - client.details.capabilities.interactive, - true, - "IClient.details.capabilities.interactive should be 'true' before resolving second container", - ); - assert.equal( - client.details.type, - undefined, - "IClient.details.type should be undefined before resolving second container", - ); - - // Resolve the container a second time with different client details. - // The contents of the [LoaderHeader.clientDetails] header end up in IContainerLoadOptions.clientDetailsOverride - // when loading the container during the loader.resolve() call. - const request: IRequest = { - headers: { - [LoaderHeader.cache]: false, - [LoaderHeader.clientDetails]: { - capabilities: { interactive: false }, - type: "myContainerType", - }, - [LoaderHeader.reconnect]: false, - }, - url: await provider.driver.createContainerUrl(documentId, container1.resolvedUrl), - }; - const container2 = await loader.resolve(request); - - // Check that the second container's client details are the expected ones - assert.equal( - (container2 as any).clientDetails?.capabilities?.interactive, - false, - "Second container's capabilities should say 'interactive: false'", - ); - assert.equal( - (container2 as any).clientDetails?.type, - "myContainerType", - "Second container's clientDetails say 'type: myContainerType'", - ); - - // Check that the first container's client details are still the expected ones after resolving the second container - assert.equal( - (container1 as any).clientDetails?.capabilities?.interactive, - true, - "First container's capabilities should say 'interactive: true' after resolving second container", - ); - assert.equal( - (container1 as any).clientDetails?.type, - undefined, - "First container's clientDetails should have undefined 'type' after resolving second container", - ); - - // Check that the IClient object passed in loader props hasn't been mutated - assert.equal( - client.details.capabilities.interactive, - true, - "IClient.details.capabilities.interactive should be 'true' after resolving second container", - ); - assert.equal( - client.details.type, - undefined, - "IClient.details.type should be undefined after resolving second container", - ); - }); }); describeNoCompat("Driver", (getTestObjectProvider) => {