From e8aa410ca398a69b6ff239ec86856bd129f2a747 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 30 Jan 2025 11:39:17 -0800 Subject: [PATCH 01/28] test(client): remove mismatched fluid-framework dep (#23682) azure-client 1.x declares a peerDependency on fluid-framework 1.4.0. This should not be elevated as normal dependency in end-to-end-tests and that dependency is removed. (Note that pnpm will allow local workspace version to satisfy that `^1.4.0` requirement, which is also a concerning pattern.) Since there is no actual peer requirement (it should have been an optional peer dependency), just remove the peer dependency when resolving dependencies using pnpm hook (`.pnpmfile.cjs`). Then also remove the end-to-end test dependency that had been added to satisfy the peer dependency. (May come back as `fluid-framework-legacy` in later change.) --- .pnpmfile.cjs | 89 +++++++++++++++++++ .../azure-client/package.json | 1 - pnpm-lock.yaml | 68 +------------- 3 files changed, 93 insertions(+), 65 deletions(-) create mode 100644 .pnpmfile.cjs diff --git a/.pnpmfile.cjs b/.pnpmfile.cjs new file mode 100644 index 000000000000..e4a6a48d9cf3 --- /dev/null +++ b/.pnpmfile.cjs @@ -0,0 +1,89 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +/* + * .pnpmfile.cjs is a pnpm hook file that allows modification of package.json content + * (in memory) of the packages that are being installed. + * https://pnpm.io/pnpmfile + * + * This implementation is based on https://gist.github.com/dvins/33b8fb52480149d37cdeb98890244c5b. + * Changes: + * - Added support for complete dependency removal (specify null for newVersion) + * - Apply stylistic and naming preferences and comment cleanup + * - Specify remapPeerDependencies for local needs + */ + +//@ts-check + +// https://pnpm.io/pnpmfile +// https://github.com/pnpm/pnpm/issues/4214 +// https://github.com/pnpm/pnpm/issues/5391 + +const remapPeerDependencies = [ + // @fluidframework/azure-client 1.x declares a peerDependency on fluid-framework but does not require it. + // It should have been an optional peer dependency. We just remove it. + { + package: "@fluidframework/azure-client", + packageVersionPrefix: "1.", + peerDependency: "fluid-framework", + newVersion: null, + }, +]; + +// Only emit the checking banner once. +// And only if engaged. Some pnpm uses expect specific output (like `pnpm list`) and may break if anything is emitted. +let emittedCheckBanner = false; + +function overridesPeerDependencies(pkg) { + if (!emittedCheckBanner) { + console.log(`Checking for package peerDependency overrides`); + emittedCheckBanner = true; + } + + if (!pkg.peerDependencies) { + return; + } + + const applicableRemapPeerDependencies = remapPeerDependencies.filter( + (remap) => + remap.package === pkg.name && pkg.version.startsWith(remap.packageVersionPrefix), + ); + + if (applicableRemapPeerDependencies.length === 0) { + return; + } + + console.log(` - Checking ${pkg.name}@${pkg.version}`); + for (const dep of applicableRemapPeerDependencies) { + if (dep.peerDependency in pkg.peerDependencies) { + console.log( + ` - Overriding ${pkg.name}@${pkg.version} peerDependency ${dep.peerDependency}@${pkg.peerDependencies[dep.peerDependency]}`, + ); + + // First add a new dependency to the package, if defined, and then remove the peer dependency. + if (dep.newVersion) { + // This approach has the added advantage that scoped overrides should now work, too. + pkg.dependencies[dep.peerDependency] = dep.newVersion; + } + delete pkg.peerDependencies[dep.peerDependency]; + + const newDep = pkg.dependencies[dep.peerDependency]; + console.log( + newDep + ? ` - Overrode ${pkg.name}@${pkg.version} peerDependency to ${dep.peerDependency}@${newDep} (as full dependency)` + : ` - Removed ${pkg.name}@${pkg.version} peerDependency`, + ); + } + } +} + +module.exports = { + hooks: { + readPackage(pkg, _context) { + overridesPeerDependencies(pkg); + return pkg; + }, + }, +}; diff --git a/packages/service-clients/end-to-end-tests/azure-client/package.json b/packages/service-clients/end-to-end-tests/azure-client/package.json index aa0cb8e5b1dd..105148ed7365 100644 --- a/packages/service-clients/end-to-end-tests/azure-client/package.json +++ b/packages/service-clients/end-to-end-tests/azure-client/package.json @@ -84,7 +84,6 @@ "@fluidframework/tree": "workspace:~", "axios": "^1.7.7", "cross-env": "^7.0.3", - "fluid-framework": "^1.4.0", "mocha": "^10.2.0", "mocha-multi-reporters": "^1.5.1", "moment": "^2.21.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a2919d7e6e91..d91deb857d23 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -18,6 +18,8 @@ overrides: oclif>@aws-sdk/client-s3: npm:empty-npm-package@1.0.0 socket.io-client: ~4.7.5 +pnpmfileChecksum: jsbnis4v7zlhs4t7yscsd3mtxe + patchedDependencies: '@microsoft/api-extractor@7.47.8': hash: ldzfpsbo3oeejrejk775zxplmi @@ -13139,7 +13141,7 @@ importers: version: link:../../azure-client '@fluidframework/azure-client-legacy': specifier: npm:@fluidframework/azure-client@^1.2.0 - version: '@fluidframework/azure-client@1.2.0(encoding@0.1.13)(fluid-framework@1.4.0)' + version: '@fluidframework/azure-client@1.2.0(encoding@0.1.13)' '@fluidframework/container-definitions': specifier: workspace:~ version: link:../../../common/container-definitions @@ -13191,9 +13193,6 @@ importers: cross-env: specifier: ^7.0.3 version: 7.0.3 - fluid-framework: - specifier: ^1.4.0 - version: 1.4.0 mocha: specifier: ^10.2.0 version: 10.8.2 @@ -17748,8 +17747,6 @@ packages: '@fluidframework/azure-client@1.2.0': resolution: {integrity: sha512-jpfYF6I1uwwCqOc8X0fOgZz4Lj91QFzKV4S9lM7jSW84GxytlL3nDanj9+EYC+vLn4liRVaOCA3zRwpkDNXRWg==} - peerDependencies: - fluid-framework: ^1.4.0 '@fluidframework/azure-client@2.20.0': resolution: {integrity: sha512-Au4TD+WwRPOSfUaEwhDeW/QroSwFeEIlJm8egoetJmeOT7eJdyFyqbr9ucsLGugBFNUfF1hngfONHk+6gMbCKw==} @@ -17908,9 +17905,6 @@ packages: '@fluidframework/matrix@2.20.0': resolution: {integrity: sha512-N3UqVg0hNtY/dkKdmf3DsWS5GknvquMzbsLAlnSW42SfI8b+I5EYpqLVPzaUv3EMMlPP6B1xNMp3Aq6j5TsOkA==} - '@fluidframework/merge-tree@1.4.0': - resolution: {integrity: sha512-UJq7AfD52bp8ecoJLxArGI8506hTFjkNYLU2TXj08Fn8pi6MYN8WBMrCgAGJ47P7ytP1YfXOOWLzeT2ODuYJYw==} - '@fluidframework/merge-tree@2.20.0': resolution: {integrity: sha512-6vdrG/rCCQE33ERfau7v0+Qxd87gBAu8PDsURCFtTewUYxz5B17MikMkpba9l850Uql3wDaLPzQKKDtTt4C6bw==} @@ -17977,9 +17971,6 @@ packages: '@fluidframework/runtime-utils@2.20.0': resolution: {integrity: sha512-QjAR9pwMqwguJ5iz1sLxr3/opfcl31rnVoX0IUkHq9bJcsjNufr+8zmZ89Yvlp0Rr4plsIqsz9Tv2fFPYGwhcg==} - '@fluidframework/sequence@1.4.0': - resolution: {integrity: sha512-KV/BNdALIgKhlivGv7U4NRCRrDWLIgWEVB2Q/oSNdKF5kUgoQnl+iIQLQcpC8dGAB7YQyCHFZVi4glanyEPF5Q==} - '@fluidframework/sequence@2.20.0': resolution: {integrity: sha512-37k91G1Yxhh9YCv0KzoLBjdi9YuKiZ4yA8mItAnXg5koS6cHJONB8kCZ7iNWIp+ygeKFavAWxIHlDvadICqZ1g==} @@ -22413,9 +22404,6 @@ packages: flatted@3.3.2: resolution: {integrity: sha512-AiwGJM8YcNOaobumgtng+6NHuOqC3A7MixFeDafM3X9cIUM+xUXoS5Vfgf+OihAYe20fxqNM9yPBXJzRtZ/4eA==} - fluid-framework@1.4.0: - resolution: {integrity: sha512-Alq2+bwmIrYBZFh/8IynI4mfHvKExLahD1wQ1/4G0PiFYVtI6k2vpRVBofXijTJYq81y3WHN7ytzycW4g/JJrQ==} - fn.name@1.1.0: resolution: {integrity: sha512-GRnmB5gPyJpAhTQdSZTSp9uaPSvl09KoYcMQtsB9rQoOmzs9dH6ffeccH+Z+cv6P68Hu5bC6JjRh4Ah/mHSNRw==} @@ -30303,7 +30291,7 @@ snapshots: - debug - supports-color - '@fluidframework/azure-client@1.2.0(encoding@0.1.13)(fluid-framework@1.4.0)': + '@fluidframework/azure-client@1.2.0(encoding@0.1.13)': dependencies: '@fluidframework/common-definitions': 0.20.1 '@fluidframework/common-utils': 1.1.1 @@ -30319,7 +30307,6 @@ snapshots: '@fluidframework/runtime-utils': 1.4.0 '@fluidframework/server-services-client': 0.1036.5002 axios: 0.28.1(debug@4.4.0) - fluid-framework: 1.4.0 uuid: 8.3.2 transitivePeerDependencies: - bufferutil @@ -31005,23 +30992,6 @@ snapshots: - debug - supports-color - '@fluidframework/merge-tree@1.4.0': - dependencies: - '@fluidframework/common-definitions': 0.20.1 - '@fluidframework/common-utils': 0.32.2 - '@fluidframework/container-definitions': 1.4.0 - '@fluidframework/container-utils': 1.4.0 - '@fluidframework/core-interfaces': 1.4.0 - '@fluidframework/datastore-definitions': 1.4.0 - '@fluidframework/protocol-definitions': 0.1028.2000 - '@fluidframework/runtime-definitions': 1.4.0 - '@fluidframework/runtime-utils': 1.4.0 - '@fluidframework/shared-object-base': 1.4.0 - '@fluidframework/telemetry-utils': 1.4.0 - transitivePeerDependencies: - - debug - - supports-color - '@fluidframework/merge-tree@2.20.0(debug@4.4.0)': dependencies: '@fluid-internal/client-utils': 2.20.0 @@ -31284,24 +31254,6 @@ snapshots: - debug - supports-color - '@fluidframework/sequence@1.4.0': - dependencies: - '@fluidframework/common-definitions': 0.20.1 - '@fluidframework/common-utils': 0.32.2 - '@fluidframework/container-utils': 1.4.0 - '@fluidframework/core-interfaces': 1.4.0 - '@fluidframework/datastore-definitions': 1.4.0 - '@fluidframework/merge-tree': 1.4.0 - '@fluidframework/protocol-definitions': 0.1028.2000 - '@fluidframework/runtime-definitions': 1.4.0 - '@fluidframework/runtime-utils': 1.4.0 - '@fluidframework/shared-object-base': 1.4.0 - '@fluidframework/telemetry-utils': 1.4.0 - uuid: 8.3.2 - transitivePeerDependencies: - - debug - - supports-color - '@fluidframework/sequence@2.20.0': dependencies: '@fluid-internal/client-utils': 2.20.0 @@ -37614,18 +37566,6 @@ snapshots: flatted@3.3.2: {} - fluid-framework@1.4.0: - dependencies: - '@fluidframework/container-definitions': 1.4.0 - '@fluidframework/container-loader': 1.4.0 - '@fluidframework/driver-definitions': 1.4.0 - '@fluidframework/fluid-static': 1.4.0 - '@fluidframework/map': 1.4.0 - '@fluidframework/sequence': 1.4.0 - transitivePeerDependencies: - - debug - - supports-color - fn.name@1.1.0: {} follow-redirects@1.15.9(debug@4.3.7): From 90eb1eaa2e86d407233d8f3b80daa3dab35f201f Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Thu, 30 Jan 2025 13:01:37 -0800 Subject: [PATCH 02/28] build(app-insights-logger): Re-enable type tests (#23695) --- .../app-insights-logger/package.json | 2 +- ...dateAppInsightsLoggerPrevious.generated.ts | 115 ++++++++++++++++++ pnpm-lock.yaml | 14 +++ 3 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 packages/framework/client-logger/app-insights-logger/src/test/types/validateAppInsightsLoggerPrevious.generated.ts diff --git a/packages/framework/client-logger/app-insights-logger/package.json b/packages/framework/client-logger/app-insights-logger/package.json index c406728bf11d..c6ce1b7ab3d3 100644 --- a/packages/framework/client-logger/app-insights-logger/package.json +++ b/packages/framework/client-logger/app-insights-logger/package.json @@ -95,6 +95,7 @@ "@biomejs/biome": "~1.9.3", "@fluid-internal/mocha-test-setup": "workspace:~", "@fluid-tools/build-cli": "^0.51.0", + "@fluidframework/app-insights-logger-previous": "npm:@fluidframework/app-insights-logger@2.20.0", "@fluidframework/build-common": "^2.0.3", "@fluidframework/build-tools": "^0.51.0", "@microsoft/api-extractor": "7.47.8", @@ -140,7 +141,6 @@ } }, "typeValidation": { - "disabled": true, "broken": {}, "entrypoint": "internal" } diff --git a/packages/framework/client-logger/app-insights-logger/src/test/types/validateAppInsightsLoggerPrevious.generated.ts b/packages/framework/client-logger/app-insights-logger/src/test/types/validateAppInsightsLoggerPrevious.generated.ts new file mode 100644 index 000000000000..7f81c620bf1c --- /dev/null +++ b/packages/framework/client-logger/app-insights-logger/src/test/types/validateAppInsightsLoggerPrevious.generated.ts @@ -0,0 +1,115 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +/* + * THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. + * Generated by flub generate:typetests in @fluid-tools/build-cli. + */ + +import type * as old from "@fluidframework/app-insights-logger-previous/internal"; +import type { TypeOnly, MinimalType, FullType, requireAssignableTo } from "@fluidframework/build-tools"; + +import type * as current from "../../index.js"; + +declare type MakeUnusedImportErrorsGoAway = TypeOnly | MinimalType | FullType | typeof old | typeof current | requireAssignableTo; + +/* + * Validate backward compatibility by using the current type in place of the old type. + * If this test starts failing, it indicates a change that is not backward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "Function_createLogger": {"backCompat": false} + */ +declare type current_as_old_for_Function_createLogger = requireAssignableTo, TypeOnly> + +/* + * Validate forward compatibility by using the old type in place of the current type. + * If this test starts failing, it indicates a change that is not forward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "Interface_CategoryFilter": {"forwardCompat": false} + */ +declare type old_as_current_for_Interface_CategoryFilter = requireAssignableTo, TypeOnly> + +/* + * Validate backward compatibility by using the current type in place of the old type. + * If this test starts failing, it indicates a change that is not backward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "Interface_CategoryFilter": {"backCompat": false} + */ +declare type current_as_old_for_Interface_CategoryFilter = requireAssignableTo, TypeOnly> + +/* + * Validate forward compatibility by using the old type in place of the current type. + * If this test starts failing, it indicates a change that is not forward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "Interface_FluidAppInsightsLoggerConfig": {"forwardCompat": false} + */ +declare type old_as_current_for_Interface_FluidAppInsightsLoggerConfig = requireAssignableTo, TypeOnly> + +/* + * Validate backward compatibility by using the current type in place of the old type. + * If this test starts failing, it indicates a change that is not backward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "Interface_FluidAppInsightsLoggerConfig": {"backCompat": false} + */ +declare type current_as_old_for_Interface_FluidAppInsightsLoggerConfig = requireAssignableTo, TypeOnly> + +/* + * Validate forward compatibility by using the old type in place of the current type. + * If this test starts failing, it indicates a change that is not forward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "Interface_NamespaceFilter": {"forwardCompat": false} + */ +declare type old_as_current_for_Interface_NamespaceFilter = requireAssignableTo, TypeOnly> + +/* + * Validate backward compatibility by using the current type in place of the old type. + * If this test starts failing, it indicates a change that is not backward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "Interface_NamespaceFilter": {"backCompat": false} + */ +declare type current_as_old_for_Interface_NamespaceFilter = requireAssignableTo, TypeOnly> + +/* + * Validate forward compatibility by using the old type in place of the current type. + * If this test starts failing, it indicates a change that is not forward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "TypeAlias_TelemetryEventCategory": {"forwardCompat": false} + */ +declare type old_as_current_for_TypeAlias_TelemetryEventCategory = requireAssignableTo, TypeOnly> + +/* + * Validate backward compatibility by using the current type in place of the old type. + * If this test starts failing, it indicates a change that is not backward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "TypeAlias_TelemetryEventCategory": {"backCompat": false} + */ +declare type current_as_old_for_TypeAlias_TelemetryEventCategory = requireAssignableTo, TypeOnly> + +/* + * Validate forward compatibility by using the old type in place of the current type. + * If this test starts failing, it indicates a change that is not forward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "TypeAlias_TelemetryFilter": {"forwardCompat": false} + */ +declare type old_as_current_for_TypeAlias_TelemetryFilter = requireAssignableTo, TypeOnly> + +/* + * Validate backward compatibility by using the current type in place of the old type. + * If this test starts failing, it indicates a change that is not backward compatible. + * To acknowledge the breaking change, add the following to package.json under + * typeValidation.broken: + * "TypeAlias_TelemetryFilter": {"backCompat": false} + */ +declare type current_as_old_for_TypeAlias_TelemetryFilter = requireAssignableTo, TypeOnly> diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d91deb857d23..3729d59c6b84 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -10978,6 +10978,9 @@ importers: '@fluid-tools/build-cli': specifier: ^0.51.0 version: 0.51.0(@types/node@18.19.67)(encoding@0.1.13)(typescript@5.4.5)(webpack-cli@5.1.4) + '@fluidframework/app-insights-logger-previous': + specifier: npm:@fluidframework/app-insights-logger@2.20.0 + version: '@fluidframework/app-insights-logger@2.20.0(tslib@1.14.1)' '@fluidframework/build-common': specifier: ^2.0.3 version: 2.0.3 @@ -17739,6 +17742,9 @@ packages: '@fluidframework/agent-scheduler@2.20.0': resolution: {integrity: sha512-8eufS3GAyYvBqz/M6aaZpj97ZJ5Q9YynYbOlHLEXi4nEYLb8QV3ike2ndoK5FpoIYZTbXgs3b3IP49yMFOlKxw==} + '@fluidframework/app-insights-logger@2.20.0': + resolution: {integrity: sha512-VjsCXnV7qLkpXwVeI/hOp6tnbFrqlQ4fUem0ci1MokoJwe+dY2PRg4bBPUJbuNpf3F8M+VepeXcVRkmm716ntw==} + '@fluidframework/aqueduct@1.4.0': resolution: {integrity: sha512-b3I3fWGAWuXQbyuEFeeEi3GVVUOYPWJfFaB4httsu5Jn4C0QSkKxftkielDlor9/7rCJYjHc4IEWViaVO+kBbA==} @@ -30249,6 +30255,14 @@ snapshots: - debug - supports-color + '@fluidframework/app-insights-logger@2.20.0(tslib@1.14.1)': + dependencies: + '@fluidframework/core-interfaces': 2.20.0 + '@microsoft/applicationinsights-web': 2.8.18(tslib@1.14.1) + '@ungap/structured-clone': 1.2.1 + transitivePeerDependencies: + - tslib + '@fluidframework/aqueduct@1.4.0': dependencies: '@fluidframework/common-definitions': 0.20.1 From abed50e12ba83d40c94613e75cb7ffcd41aee354 Mon Sep 17 00:00:00 2001 From: Tommy Brosman Date: Thu, 30 Jan 2025 13:59:29 -0800 Subject: [PATCH 03/28] Replace IsomorphicPerformance with a performanceNow() method (#23668) IsomorphicPerformance was an internal API in client-utils. It included a number of methods supported by the [Performance](https://developer.mozilla.org/en-US/docs/Web/API/Performance) API, including `mark()` and `measure()`. The only member of the API that was used as of this change is `now()`. This change removes IsomorphicPerformance, exposes `performanceNow()`, and reduces the amount of work needed to restructure client-utils. Note that there is another copy of IsomorphicPerformance in common-utils. The common-utils version is deprecated, so this PR does not change it. --- .../api-report/client-utils.beta.api.md | 2 -- .../api-report/client-utils.public.api.md | 2 -- packages/common/client-utils/package.json | 10 ++++++- .../common/client-utils/src/indexBrowser.ts | 3 +- packages/common/client-utils/src/indexNode.ts | 3 +- .../client-utils/src/performanceIsomorphic.ts | 20 +++++-------- .../validateClientUtilsPrevious.generated.ts | 3 ++ packages/common/client-utils/src/trace.ts | 6 ++-- .../drivers/driver-base/src/driverUtils.ts | 3 +- .../src/odspDelayLoadedDeltaStream.ts | 6 ++-- .../src/odspDocumentDeltaConnection.ts | 8 +++--- .../src/odspDocumentStorageManager.ts | 14 +++++----- packages/drivers/odsp-driver/src/odspUtils.ts | 14 +++++----- .../drivers/odsp-driver/src/opsCaching.ts | 6 ++-- .../odsp-driver/src/prefetchLatestSnapshot.ts | 4 +-- .../drivers/odsp-driver/src/retryUtils.ts | 10 +++---- .../routerlicious-driver/src/restWrapper.ts | 14 +++++----- .../src/wholeSummaryDocumentStorageService.ts | 10 +++++-- .../container-loader/src/connectionManager.ts | 16 +++++------ .../loader/container-loader/src/container.ts | 28 +++++++++---------- .../container-loader/src/debugLogger.ts | 4 +-- .../loader/container-loader/src/deltaQueue.ts | 6 ++-- .../driver-utils/src/parallelRequests.ts | 14 +++++----- .../loader/driver-utils/src/runWithRetry.ts | 12 ++++---- .../container-runtime/src/batchTracker.ts | 4 +-- .../src/connectionTelemetry.ts | 8 +++--- .../container-runtime/src/deltaScheduler.ts | 12 ++++---- .../src/inboundBatchAggregator.ts | 6 ++-- packages/utils/telemetry-utils/src/logger.ts | 6 ++-- .../src/sampledTelemetryHelper.ts | 6 ++-- 30 files changed, 131 insertions(+), 129 deletions(-) diff --git a/packages/common/client-utils/api-report/client-utils.beta.api.md b/packages/common/client-utils/api-report/client-utils.beta.api.md index da59d647b5c2..6a885f156756 100644 --- a/packages/common/client-utils/api-report/client-utils.beta.api.md +++ b/packages/common/client-utils/api-report/client-utils.beta.api.md @@ -6,8 +6,6 @@ export { EventEmitter } -export { performance_2 as performance } - // (No @packageDocumentation comment for this package) ``` diff --git a/packages/common/client-utils/api-report/client-utils.public.api.md b/packages/common/client-utils/api-report/client-utils.public.api.md index db4e6735fad2..ba710ff382bf 100644 --- a/packages/common/client-utils/api-report/client-utils.public.api.md +++ b/packages/common/client-utils/api-report/client-utils.public.api.md @@ -6,8 +6,6 @@ export { EventEmitter } -export { performance_2 as performance } - // (No @packageDocumentation comment for this package) ``` diff --git a/packages/common/client-utils/package.json b/packages/common/client-utils/package.json index d130369bd4fa..b4d527140845 100644 --- a/packages/common/client-utils/package.json +++ b/packages/common/client-utils/package.json @@ -244,7 +244,15 @@ } }, "typeValidation": { - "broken": {}, + "broken": { + "Variable_performance": { + "backCompat": false + }, + "TypeAlias_IsomorphicPerformance": { + "forwardCompat": false, + "backCompat": false + } + }, "entrypoint": "public" } } diff --git a/packages/common/client-utils/src/indexBrowser.ts b/packages/common/client-utils/src/indexBrowser.ts index a7933a4f5659..8ca0a19404e0 100644 --- a/packages/common/client-utils/src/indexBrowser.ts +++ b/packages/common/client-utils/src/indexBrowser.ts @@ -14,12 +14,11 @@ export { Uint8ArrayToString, } from "./bufferBrowser.js"; export { gitHashFile, hashFile } from "./hashFileBrowser.js"; -export { performance } from "./performanceIsomorphic.js"; export { fromBase64ToUtf8, fromUtf8ToBase64, toUtf8 } from "./base64EncodingBrowser.js"; export { Uint8ArrayToArrayBuffer } from "./bufferShared.js"; export { EventEmitter } from "./eventEmitter.cjs"; -export { type IsomorphicPerformance } from "./performanceIsomorphic.js"; +export { performanceNow } from "./performanceIsomorphic.js"; export { type ITraceEvent, Trace } from "./trace.js"; export { type EventEmitterEventType, diff --git a/packages/common/client-utils/src/indexNode.ts b/packages/common/client-utils/src/indexNode.ts index 8590bb62b967..066be7f7aae2 100644 --- a/packages/common/client-utils/src/indexNode.ts +++ b/packages/common/client-utils/src/indexNode.ts @@ -14,12 +14,11 @@ export { Uint8ArrayToString, } from "./bufferNode.js"; export { gitHashFile, hashFile } from "./hashFileNode.js"; -export { performance } from "./performanceIsomorphic.js"; export { fromBase64ToUtf8, fromUtf8ToBase64, toUtf8 } from "./base64EncodingNode.js"; export { Uint8ArrayToArrayBuffer } from "./bufferShared.js"; export { EventEmitter } from "./eventEmitter.cjs"; -export type { IsomorphicPerformance } from "./performanceIsomorphic.js"; +export { performanceNow } from "./performanceIsomorphic.js"; export { type ITraceEvent, Trace } from "./trace.js"; export { type EventEmitterEventType, diff --git a/packages/common/client-utils/src/performanceIsomorphic.ts b/packages/common/client-utils/src/performanceIsomorphic.ts index 3aa95e8f2d74..55639e135b4c 100644 --- a/packages/common/client-utils/src/performanceIsomorphic.ts +++ b/packages/common/client-utils/src/performanceIsomorphic.ts @@ -4,20 +4,14 @@ */ /** - * This type contains all browser performance properties as optional, and some - * of the intersecting properties of node and browser performance as required. + * Exposes `Performance.now()` in both Node and browser environments. * - * @internal - */ -export type IsomorphicPerformance = Partial & - Pick; - -/** - * This exported "performance" member masks the built-in globalThis.performance object - * as an IsomorphicPerformance, which hides all of its features that aren't compatible - * between Node and browser implementations. Anything exposed on this performance object - * is considered safe to use regarless of the environment it runs in. + * @remarks + * + * The performance API is available as an attribute on the `WindowOrWorkerGlobalScope` object which `globalThis` points to. + * - The [global `performance` attribute](https://w3c.github.io/hr-time/#the-performance-attribute) + * - [`globalThis`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/globalThis) * * @internal */ -export const performance: IsomorphicPerformance = globalThis.performance; +export const performanceNow: () => number = () => globalThis.performance.now(); diff --git a/packages/common/client-utils/src/test/types/validateClientUtilsPrevious.generated.ts b/packages/common/client-utils/src/test/types/validateClientUtilsPrevious.generated.ts index 033c49b12e48..ad64e0372120 100644 --- a/packages/common/client-utils/src/test/types/validateClientUtilsPrevious.generated.ts +++ b/packages/common/client-utils/src/test/types/validateClientUtilsPrevious.generated.ts @@ -238,6 +238,7 @@ declare type current_as_old_for_TypeAlias_IsoBuffer = requireAssignableTo, TypeOnly> /* @@ -247,6 +248,7 @@ declare type old_as_current_for_TypeAlias_IsomorphicPerformance = requireAssigna * typeValidation.broken: * "TypeAlias_IsomorphicPerformance": {"backCompat": false} */ +// @ts-expect-error compatibility expected to be broken declare type current_as_old_for_TypeAlias_IsomorphicPerformance = requireAssignableTo, TypeOnly> /* @@ -310,6 +312,7 @@ declare type current_as_old_for_Variable_IsoBuffer = requireAssignableTo, TypeOnly> /* diff --git a/packages/common/client-utils/src/trace.ts b/packages/common/client-utils/src/trace.ts index 86d754fb28d8..4b71a568fb82 100644 --- a/packages/common/client-utils/src/trace.ts +++ b/packages/common/client-utils/src/trace.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "./performanceIsomorphic.js"; +import { performanceNow } from "./performanceIsomorphic.js"; /** * Helper class for tracing performance of events @@ -13,7 +13,7 @@ import { performance } from "./performanceIsomorphic.js"; */ export class Trace { public static start(): Trace { - const startTick = performance.now(); + const startTick = performanceNow(); return new Trace(startTick); } @@ -23,7 +23,7 @@ export class Trace { } public trace(): ITraceEvent { - const tick = performance.now(); + const tick = performanceNow(); const event = { totalTimeElapsed: tick - this.startTick, duration: tick - this.lastTick, diff --git a/packages/drivers/driver-base/src/driverUtils.ts b/packages/drivers/driver-base/src/driverUtils.ts index c98cfc862a35..11c1e8b1c485 100644 --- a/packages/drivers/driver-base/src/driverUtils.ts +++ b/packages/drivers/driver-base/src/driverUtils.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; import { ISequencedDocumentMessage } from "@fluidframework/driver-definitions/internal"; import { ITelemetryLoggerExt } from "@fluidframework/telemetry-utils/internal"; @@ -48,7 +47,7 @@ export function getW3CData(url: string, initiatorType: string) { let w3cStartTime: number | undefined; // W3C Start time = fetchStart time // getEntriesByType is only available in browser performance object - const resources1 = performance.getEntriesByType?.("resource") ?? []; + const resources1 = globalThis.performance.getEntriesByType?.("resource") ?? []; // Usually the latest fetch call is to the end of resources, so we start from the end. for (let i = resources1.length - 1; i > 0; i--) { const indResTime = resources1[i] as PerformanceResourceTiming; diff --git a/packages/drivers/odsp-driver/src/odspDelayLoadedDeltaStream.ts b/packages/drivers/odsp-driver/src/odspDelayLoadedDeltaStream.ts index 05bcdab8b4ee..817798bb6481 100644 --- a/packages/drivers/odsp-driver/src/odspDelayLoadedDeltaStream.ts +++ b/packages/drivers/odsp-driver/src/odspDelayLoadedDeltaStream.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { ISignalEnvelope } from "@fluidframework/core-interfaces/internal"; import { assert } from "@fluidframework/core-utils/internal"; import { IClient } from "@fluidframework/driver-definitions"; @@ -519,7 +519,7 @@ export class OdspDelayLoadedDeltaStream { client: IClient, webSocketUrl: string, ): Promise { - const startTime = performance.now(); + const startTime = performanceNow(); const connection = await OdspDocumentDeltaConnection.create( tenantId, documentId, @@ -531,7 +531,7 @@ export class OdspDelayLoadedDeltaStream { this.epochTracker, this.socketReferenceKeyPrefix, ); - const duration = performance.now() - startTime; + const duration = performanceNow() - startTime; // This event happens rather often, so it adds up to cost of telemetry. // Given that most reconnects result in reusing socket and happen very quickly, // report event only if it took longer than threshold. diff --git a/packages/drivers/odsp-driver/src/odspDocumentDeltaConnection.ts b/packages/drivers/odsp-driver/src/odspDocumentDeltaConnection.ts index 7c185dc64dfd..980aa1ea04e5 100644 --- a/packages/drivers/odsp-driver/src/odspDocumentDeltaConnection.ts +++ b/packages/drivers/odsp-driver/src/odspDocumentDeltaConnection.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { TypedEventEmitter, performance } from "@fluid-internal/client-utils"; +import { TypedEventEmitter, performanceNow } from "@fluid-internal/client-utils"; import { IEvent } from "@fluidframework/core-interfaces"; import { assert, Deferred } from "@fluidframework/core-utils/internal"; import { DocumentDeltaConnection } from "@fluidframework/driver-base/internal"; @@ -450,7 +450,7 @@ export class OdspDocumentDeltaConnection extends DocumentDeltaConnection { this.pushCallCounter++; const nonce = `${this.requestOpsNoncePrefix}${this.pushCallCounter}`; - const start = performance.now(); + const start = performanceNow(); // We may keep keep accumulating memory for nothing, if we are not getting responses. // Note that we should not have overlapping requests, as DeltaManager allows only one @@ -475,7 +475,7 @@ export class OdspDocumentDeltaConnection extends DocumentDeltaConnection { from: payloadToDelete.from, to: payloadToDelete.to, length: payloadToDelete.to - payloadToDelete.from, - duration: performance.now() - payloadToDelete.start, + duration: performanceNow() - payloadToDelete.start, }); this.getOpsMap.delete(key!); } @@ -591,7 +591,7 @@ export class OdspDocumentDeltaConnection extends DocumentDeltaConnection { code: result.code, from: data?.from, to: data?.to, - duration: data === undefined ? undefined : performance.now() - data.start, + duration: data === undefined ? undefined : performanceNow() - data.start, }; if (messages !== undefined && messages.length > 0) { this.logger.sendPerformanceEvent({ diff --git a/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts b/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts index db592827df0b..ae15139f1b26 100644 --- a/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts +++ b/packages/drivers/odsp-driver/src/odspDocumentStorageManager.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { LogLevel } from "@fluidframework/core-interfaces"; import { assert, delay } from "@fluidframework/core-utils/internal"; import { promiseRaceWithWinner } from "@fluidframework/driver-base/internal"; @@ -271,7 +271,7 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { let retrievedSnapshot: ISnapshot | IPrefetchSnapshotContents | undefined; let method: string; - let prefetchWaitStartTime: number = performance.now(); + let prefetchWaitStartTime: number = performanceNow(); if (snapshotFetchOptions.fetchSource === FetchSource.noCache) { retrievedSnapshot = await this.fetchSnapshotFromNetwork( hostSnapshotOptions, @@ -394,13 +394,13 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { } else { // Note: There's a race condition here - another caller may come past the undefined check // while the first caller is awaiting later async code in this block. - const startTime = performance.now(); + const startTime = performanceNow(); retrievedSnapshot = await cachedSnapshotP; - cacheLookupTimeInSerialFetch = performance.now() - startTime; + cacheLookupTimeInSerialFetch = performanceNow() - startTime; method = retrievedSnapshot === undefined ? "network" : "cache"; if (retrievedSnapshot === undefined) { - prefetchWaitStartTime = performance.now(); + prefetchWaitStartTime = performanceNow(); retrievedSnapshot = await this.fetchSnapshotFromNetwork( hostSnapshotOptions, snapshotFetchOptions.loadingGroupIds, @@ -437,7 +437,7 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { }, ); - const stTime = performance.now(); + const stTime = performanceNow(); // Don't override ops which were fetched during initial load, since we could still need them. const id = this.initializeFromSnapshot( odspSnapshotCacheValue, @@ -447,7 +447,7 @@ export class OdspDocumentStorageService extends OdspDocumentStorageServiceBase { this.logger.sendTelemetryEvent( { eventName: "SnapshotInitializeTime", - duration: performance.now() - stTime, + duration: performanceNow() - stTime, }, undefined, LogLevel.verbose, diff --git a/packages/drivers/odsp-driver/src/odspUtils.ts b/packages/drivers/odsp-driver/src/odspUtils.ts index 8bc47ae41dfa..5da0ac75b809 100644 --- a/packages/drivers/odsp-driver/src/odspUtils.ts +++ b/packages/drivers/odsp-driver/src/odspUtils.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { ITelemetryBaseLogger, ITelemetryBaseProperties, @@ -135,7 +135,7 @@ export async function fetchHelper( requestInfo: RequestInfo, requestInit: RequestInit | undefined, ): Promise> { - const start = performance.now(); + const start = performanceNow(); // Node-fetch and dom have conflicting typing, force them to work by casting for now return fetch(requestInfo, requestInit).then( @@ -165,7 +165,7 @@ export async function fetchHelper( content: response, headers, propsToLog: getSPOAndGraphRequestIdsFromResponse(headers), - duration: performance.now() - start, + duration: performanceNow() - start, }; }, (error) => { @@ -508,16 +508,16 @@ export function buildOdspShareLinkReqParams( } export function measure(callback: () => T): [T, number] { - const start = performance.now(); + const start = performanceNow(); const result = callback(); - const time = performance.now() - start; + const time = performanceNow() - start; return [result, time]; } export async function measureP(callback: () => Promise): Promise<[T, number]> { - const start = performance.now(); + const start = performanceNow(); const result = await callback(); - const time = performance.now() - start; + const time = performanceNow() - start; return [result, time]; } diff --git a/packages/drivers/odsp-driver/src/opsCaching.ts b/packages/drivers/odsp-driver/src/opsCaching.ts index b2d04db44403..80347d2028db 100644 --- a/packages/drivers/odsp-driver/src/opsCaching.ts +++ b/packages/drivers/odsp-driver/src/opsCaching.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { ITelemetryLoggerExt } from "@fluidframework/telemetry-utils/internal"; // ISequencedDocumentMessage @@ -183,11 +183,11 @@ export class OpsCache { * @returns ops retrieved */ public async get(from: number, to?: number): Promise { - const start = performance.now(); + const start = performanceNow(); const messages = await this.getCore(from, to); - const duration = performance.now() - start; + const duration = performanceNow() - start; if (messages.length > 0 || duration > 1000) { this.logger.sendPerformanceEvent({ eventName: "CacheOpsUsed", diff --git a/packages/drivers/odsp-driver/src/prefetchLatestSnapshot.ts b/packages/drivers/odsp-driver/src/prefetchLatestSnapshot.ts index 53f2e626554a..ad48a594a8a1 100644 --- a/packages/drivers/odsp-driver/src/prefetchLatestSnapshot.ts +++ b/packages/drivers/odsp-driver/src/prefetchLatestSnapshot.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { ITelemetryBaseLogger } from "@fluidframework/core-interfaces"; import { assert, Deferred } from "@fluidframework/core-utils/internal"; import { IResolvedUrl } from "@fluidframework/driver-definitions/internal"; @@ -127,7 +127,7 @@ export async function prefetchLatestSnapshot( odspLogger, { eventName: "PrefetchLatestSnapshot" }, async () => { - const prefetchStartTime = performance.now(); + const prefetchStartTime = performanceNow(); // Add the deferred promise to the cache, so that it can be leveraged while loading the container. const snapshotContentsWithEpochP = new Deferred(); const nonPersistentCacheKey = getKeyForCacheEntry(snapshotKey); diff --git a/packages/drivers/odsp-driver/src/retryUtils.ts b/packages/drivers/odsp-driver/src/retryUtils.ts index c413bdd134b0..2296108995ea 100644 --- a/packages/drivers/odsp-driver/src/retryUtils.ts +++ b/packages/drivers/odsp-driver/src/retryUtils.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { delay } from "@fluidframework/core-utils/internal"; import { canRetryOnError, @@ -24,7 +24,7 @@ export async function runWithRetry( checkDisposed?: () => void, ): Promise { let retryAfter = 1000; - const start = performance.now(); + const start = performanceNow(); let lastError: unknown; for (let attempts = 1; ; attempts++) { if (checkDisposed !== undefined) { @@ -38,7 +38,7 @@ export async function runWithRetry( eventName: "MultipleRetries", callName, attempts, - duration: performance.now() - start, + duration: performanceNow() - start, }, lastError, ); @@ -62,7 +62,7 @@ export async function runWithRetry( eventName: `${callName}_firstFailed`, callName, attempts, - duration: performance.now() - start, // record total wait time. + duration: performanceNow() - start, // record total wait time. }, error, ); @@ -86,7 +86,7 @@ export async function runWithRetry( : "ServiceReadonlyErrorTooManyRetries", callName, attempts, - duration: performance.now() - start, // record total wait time. + duration: performanceNow() - start, // record total wait time. }, error, ); diff --git a/packages/drivers/routerlicious-driver/src/restWrapper.ts b/packages/drivers/routerlicious-driver/src/restWrapper.ts index e53fc58200b3..d481a9dbebc2 100644 --- a/packages/drivers/routerlicious-driver/src/restWrapper.ts +++ b/packages/drivers/routerlicious-driver/src/restWrapper.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { fromUtf8ToBase64, performance } from "@fluid-internal/client-utils"; +import { fromUtf8ToBase64, performanceNow } from "@fluid-internal/client-utils"; import { ITelemetryBaseProperties } from "@fluidframework/core-interfaces"; import { assert } from "@fluidframework/core-utils/internal"; import { @@ -172,7 +172,7 @@ class RouterliciousRestWrapper extends RestWrapper { const fetchRequestConfig = axiosBuildRequestInitConfig(translatedConfig); const res = await this.rateLimiter.schedule(async () => { - const perfStart = performance.now(); + const perfStart = performanceNow(); const result = await fetch(completeRequestUrl, fetchRequestConfig).catch( async (error) => { // on failure, add the request entry into the retryCounter map to count the subsequent retries, if any @@ -211,25 +211,25 @@ class RouterliciousRestWrapper extends RestWrapper { ); return { response: result, - duration: performance.now() - perfStart, + duration: performanceNow() - perfStart, }; }); const response = res.response; const headers = headersToMap(response.headers); - let start = performance.now(); + let start = performanceNow(); const text = await response.text(); - const receiveContentTime = performance.now() - start; + const receiveContentTime = performanceNow() - start; const bodySize = text.length; - start = performance.now(); + start = performanceNow(); const responseBody: any = response.headers .get("content-type") ?.includes("application/json") ? JSON.parse(text) : text; - const parseTime = performance.now() - start; + const parseTime = performanceNow() - start; // Success if (response.ok || response.status === statusCode) { diff --git a/packages/drivers/routerlicious-driver/src/wholeSummaryDocumentStorageService.ts b/packages/drivers/routerlicious-driver/src/wholeSummaryDocumentStorageService.ts index f722d842a413..336a1fe89912 100644 --- a/packages/drivers/routerlicious-driver/src/wholeSummaryDocumentStorageService.ts +++ b/packages/drivers/routerlicious-driver/src/wholeSummaryDocumentStorageService.ts @@ -3,7 +3,11 @@ * Licensed under the MIT License. */ -import { Uint8ArrayToString, performance, stringToBuffer } from "@fluid-internal/client-utils"; +import { + Uint8ArrayToString, + performanceNow, + stringToBuffer, +} from "@fluid-internal/client-utils"; import { assert } from "@fluidframework/core-utils/internal"; import { getW3CData, promiseRaceWithWinner } from "@fluidframework/driver-base/internal"; import { ISummaryHandle, ISummaryTree } from "@fluidframework/driver-definitions"; @@ -306,10 +310,10 @@ export class WholeSummaryDocumentStorageService implements IDocumentStorageServi const manager = await this.getStorageManager(disableCache); const response: IR11sResponse = await manager.getSnapshot(versionId); - const start = performance.now(); + const start = performanceNow(); const snapshot: INormalizedWholeSnapshot = convertWholeFlatSnapshotToSnapshotTreeAndBlobs(response.content); - const snapshotConversionTime = performance.now() - start; + const snapshotConversionTime = performanceNow() - start; validateBlobsAndTrees(snapshot.snapshotTree); const { trees, numBlobs, encodedBlobsSize } = evalBlobsAndTrees(snapshot); diff --git a/packages/loader/container-loader/src/connectionManager.ts b/packages/loader/container-loader/src/connectionManager.ts index 711e00e8d25b..20e4703d0ad5 100644 --- a/packages/loader/container-loader/src/connectionManager.ts +++ b/packages/loader/container-loader/src/connectionManager.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { TypedEventEmitter, performance } from "@fluid-internal/client-utils"; +import { TypedEventEmitter, performanceNow } from "@fluid-internal/client-utils"; import { ICriticalContainerError } from "@fluidframework/container-definitions"; import { IDeltaQueue, ReadOnlyInfo } from "@fluidframework/container-definitions/internal"; import { @@ -583,7 +583,7 @@ export class ConnectionManager implements IConnectionManager { let delayMs = InitialReconnectDelayInMs; let connectRepeatCount = 0; - const connectStartTime = performance.now(); + const connectStartTime = performanceNow(); let lastError: unknown; const abortController = new AbortController(); @@ -604,7 +604,7 @@ export class ConnectionManager implements IConnectionManager { this.logger.sendTelemetryEvent({ eventName: "ConnectionAttemptCancelled", attempts: connectRepeatCount, - duration: formatTick(performance.now() - connectStartTime), + duration: formatTick(performanceNow() - connectStartTime), connectionEstablished: false, }); return; @@ -675,7 +675,7 @@ export class ConnectionManager implements IConnectionManager { attempts: connectRepeatCount, delay: delayMs, // milliseconds eventName: "DeltaConnectionFailureToConnect", - duration: formatTick(performance.now() - connectStartTime), + duration: formatTick(performanceNow() - connectStartTime), }, origError, ); @@ -688,7 +688,7 @@ export class ConnectionManager implements IConnectionManager { return; } - const waitStartTime = performance.now(); + const waitStartTime = performanceNow(); const retryDelayFromError = getRetryDelayFromError(origError); // If the error told us to wait or browser signals us that we are offline, then calculate the time we // want to wait for before retrying. then we wait for that time. If the error didn't tell us to wait, @@ -714,7 +714,7 @@ export class ConnectionManager implements IConnectionManager { await waitForOnline(); this.logger.sendPerformanceEvent({ eventName: "WaitBetweenConnectionAttempts", - duration: performance.now() - waitStartTime, + duration: performanceNow() - waitStartTime, details: JSON.stringify({ retryDelayFromError, delayMs, @@ -730,7 +730,7 @@ export class ConnectionManager implements IConnectionManager { { eventName: "MultipleDeltaConnectionFailures", attempts: connectRepeatCount, - duration: formatTick(performance.now() - connectStartTime), + duration: formatTick(performanceNow() - connectStartTime), }, lastError, ); @@ -742,7 +742,7 @@ export class ConnectionManager implements IConnectionManager { this.logger.sendTelemetryEvent({ eventName: "ConnectionAttemptCancelled", attempts: connectRepeatCount, - duration: formatTick(performance.now() - connectStartTime), + duration: formatTick(performanceNow() - connectStartTime), connectionEstablished: true, }); return; diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index f8c24ff506eb..6d81eb162ba3 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -7,7 +7,7 @@ import { TypedEventEmitter, - performance, + performanceNow, type ILayerCompatDetails, } from "@fluid-internal/client-utils"; import { @@ -622,7 +622,7 @@ export class Container private readonly clientsWhoShouldHaveLeft = new Set(); private _containerMetadata: Readonly> = {}; - private setAutoReconnectTime = performance.now(); + private setAutoReconnectTime = performanceNow(); private noopHeuristic: NoopHeuristic | undefined; @@ -808,7 +808,7 @@ export class Container protocolHandlerBuilder, } = createProps; - this.connectionTransitionTimes[ConnectionState.Disconnected] = performance.now(); + this.connectionTransitionTimes[ConnectionState.Disconnected] = performanceNow(); const pendingLocalState = loadProps?.pendingLocalState; this._canReconnect = canReconnect ?? true; @@ -896,7 +896,7 @@ export class Container : this.deltaManager?.lastMessage?.clientId, dmLastMsgClientSeq: () => this.deltaManager?.lastMessage?.clientSequenceNumber, connectionStateDuration: () => - performance.now() - this.connectionTransitionTimes[this.connectionState], + performanceNow() - this.connectionTransitionTimes[this.connectionState], }, }, }); @@ -940,7 +940,7 @@ export class Container mode, category: this._lifecycleState === "loading" ? "generic" : category, duration: - performance.now() - this.connectionTransitionTimes[ConnectionState.CatchingUp], + performanceNow() - this.connectionTransitionTimes[ConnectionState.CatchingUp], ...(details === undefined ? {} : { details: JSON.stringify(details) }), }); @@ -1036,10 +1036,10 @@ export class Container document.addEventListener !== null; // keep track of last time page was visible for telemetry (on interactive clients only) if (isDomAvailable && interactive) { - this.lastVisible = document.hidden ? performance.now() : undefined; + this.lastVisible = document.hidden ? performanceNow() : undefined; this.visibilityEventHandler = (): void => { if (document.hidden) { - this.lastVisible = performance.now(); + this.lastVisible = performanceNow(); } else { // settimeout so this will hopefully fire after disconnect event if being hidden caused it setTimeout(() => { @@ -1416,7 +1416,7 @@ export class Container return; } - const now = performance.now(); + const now = performanceNow(); const duration = now - this.setAutoReconnectTime; this.setAutoReconnectTime = now; @@ -1634,7 +1634,7 @@ export class Container dmLastProcessedSeqNumber: number; dmLastKnownSeqNumber: number; }> { - const timings: Record = { phase1: performance.now() }; + const timings: Record = { phase1: performanceNow() }; this.service = await this.createDocumentService(async () => this.serviceFactory.createDocumentService( resolvedUrl, @@ -1667,7 +1667,7 @@ export class Container state: AttachState.Attached, }; - timings.phase2 = performance.now(); + timings.phase2 = performanceNow(); // Fetch specified snapshot. const { baseSnapshot, version } = @@ -1727,7 +1727,7 @@ export class Container this.protocolHandler.audience.setCurrentClientId(pendingLocalState?.clientId); } - timings.phase3 = performance.now(); + timings.phase3 = performanceNow(); const codeDetails = this.getCodeDetailsFromQuorum(); await this.instantiateRuntime( codeDetails, @@ -1790,7 +1790,7 @@ export class Container throw new Error("Container was closed while load()"); } - timings.end = performance.now(); + timings.end = performanceNow(); this.subLogger.sendTelemetryEvent( { eventName: "LoadStagesTimings", @@ -2157,7 +2157,7 @@ export class Container reason?: IConnectionStateChangeReason, ): void { // Log actual event - const time = performance.now(); + const time = performanceNow(); this.connectionTransitionTimes[value] = time; const duration = time - this.connectionTransitionTimes[oldState]; @@ -2196,7 +2196,7 @@ export class Container opsBehind, online: OnlineStatus[isOnline()], lastVisible: - this.lastVisible === undefined ? undefined : performance.now() - this.lastVisible, + this.lastVisible === undefined ? undefined : performanceNow() - this.lastVisible, checkpointSequenceNumber, quorumSize: this._protocolHandler?.quorum.getMembers().size, audienceSize: this._protocolHandler?.audience.getMembers().size, diff --git a/packages/loader/container-loader/src/debugLogger.ts b/packages/loader/container-loader/src/debugLogger.ts index c7d2165ca5bc..ba6746b5b6b8 100644 --- a/packages/loader/container-loader/src/debugLogger.ts +++ b/packages/loader/container-loader/src/debugLogger.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { ITelemetryBaseEvent, ITelemetryBaseLogger, @@ -88,7 +88,7 @@ export class DebugLogger implements ITelemetryBaseLogger { newEvent.eventName = undefined; let tick = ""; - tick = `tick=${formatTick(performance.now())}`; + tick = `tick=${formatTick(performanceNow())}`; // Extract stack to put it last, but also to avoid escaping '\n' in it by JSON.stringify below const stack = newEvent.stack ?? ""; diff --git a/packages/loader/container-loader/src/deltaQueue.ts b/packages/loader/container-loader/src/deltaQueue.ts index 598b66ba6f8a..78283db09f01 100644 --- a/packages/loader/container-loader/src/deltaQueue.ts +++ b/packages/loader/container-loader/src/deltaQueue.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { TypedEventEmitter, performance } from "@fluid-internal/client-utils"; +import { TypedEventEmitter, performanceNow } from "@fluid-internal/client-utils"; import { IDeltaQueue, IDeltaQueueEvents, @@ -153,7 +153,7 @@ export class DeltaQueue count: number; duration: number; } { - const start = performance.now(); + const start = performanceNow(); let count = 0; // For grouping to work we must process all local messages immediately and in the single turn. @@ -169,7 +169,7 @@ export class DeltaQueue this.emit("op", next); } - const duration = performance.now() - start; + const duration = performanceNow() - start; if (this.q.length === 0) { this.emit("idle", count, duration); } diff --git a/packages/loader/driver-utils/src/parallelRequests.ts b/packages/loader/driver-utils/src/parallelRequests.ts index bc9f9b3ff013..5df8861d808a 100644 --- a/packages/loader/driver-utils/src/parallelRequests.ts +++ b/packages/loader/driver-utils/src/parallelRequests.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { ITelemetryBaseProperties } from "@fluidframework/core-interfaces"; import { assert, Deferred } from "@fluidframework/core-utils/internal"; import { @@ -436,7 +436,7 @@ async function getSingleOpBatch( while (signal?.aborted !== true) { retry++; let lastError: unknown; - const startTime = performance.now(); + const startTime = performanceNow(); try { // Issue async request for deltas @@ -459,8 +459,8 @@ async function getSingleOpBatch( if (lastSuccessTime === undefined) { // Take timestamp of the first time server responded successfully, even though it wasn't with the ops we asked for. // If we keep getting empty responses we'll eventually fail out below. - lastSuccessTime = performance.now(); - } else if (performance.now() - lastSuccessTime > 30000) { + lastSuccessTime = performanceNow(); + } else if (performanceNow() - lastSuccessTime > 30000) { // If we are connected and receiving proper responses from server, but can't get any ops back, // then give up after some time. This likely indicates the issue with ordering service not flushing // ops to storage quick enough, and possibly waiting for summaries, while summarizer can't get @@ -489,7 +489,7 @@ async function getSingleOpBatch( eventName: "GetDeltas_Error", ...props, retry, - duration: performance.now() - startTime, + duration: performanceNow() - startTime, retryAfter, reason: scenarioName, }, @@ -503,7 +503,7 @@ async function getSingleOpBatch( } if (telemetryEvent === undefined) { - waitStartTime = performance.now(); + waitStartTime = performanceNow(); telemetryEvent = PerformanceEvent.start(logger, { eventName: "GetDeltasWaitTime", }); @@ -521,7 +521,7 @@ async function getSingleOpBatch( // NOTE: This isn't strictly true for drivers that don't require network (e.g. local driver). Really this logic // should probably live in the driver. await waitForOnline(); - totalRetryAfterTime += performance.now() - waitStartTime; + totalRetryAfterTime += performanceNow() - waitStartTime; } return nothing; diff --git a/packages/loader/driver-utils/src/runWithRetry.ts b/packages/loader/driver-utils/src/runWithRetry.ts index fe7681a0f222..9dfc2ff6adf9 100644 --- a/packages/loader/driver-utils/src/runWithRetry.ts +++ b/packages/loader/driver-utils/src/runWithRetry.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { delay } from "@fluidframework/core-utils/internal"; import { DriverErrorTypes } from "@fluidframework/driver-definitions/internal"; import { ITelemetryLoggerExt, isFluidError } from "@fluidframework/telemetry-utils/internal"; @@ -58,7 +58,7 @@ export async function runWithRetry( // We double this value in first try in when we calculate time to wait for in "calculateMaxWaitTime" function. let retryAfterMs = 500; // has to be positive! let numRetries = 0; - const startTime = performance.now(); + const startTime = performanceNow(); let lastError: any; do { try { @@ -71,7 +71,7 @@ export async function runWithRetry( { eventName: `${fetchCallName}_cancel`, retry: numRetries, - duration: performance.now() - startTime, + duration: performanceNow() - startTime, fetchCallName, }, err, @@ -84,7 +84,7 @@ export async function runWithRetry( { eventName: `${fetchCallName}_runWithRetryAborted`, retry: numRetries, - duration: performance.now() - startTime, + duration: performanceNow() - startTime, fetchCallName, reason: progress.cancel.reason, }, @@ -108,7 +108,7 @@ export async function runWithRetry( logger.sendTelemetryEvent( { eventName: `${fetchCallName}_firstFailed`, - duration: performance.now() - startTime, + duration: performanceNow() - startTime, fetchCallName, }, err, @@ -130,7 +130,7 @@ export async function runWithRetry( { eventName: `${fetchCallName}_lastError`, retry: numRetries, - duration: performance.now() - startTime, + duration: performanceNow() - startTime, fetchCallName, }, lastError, diff --git a/packages/runtime/container-runtime/src/batchTracker.ts b/packages/runtime/container-runtime/src/batchTracker.ts index 5eaa77a41e03..d8f4ba8cce07 100644 --- a/packages/runtime/container-runtime/src/batchTracker.ts +++ b/packages/runtime/container-runtime/src/batchTracker.ts @@ -4,7 +4,7 @@ */ import type { EventEmitter } from "@fluid-internal/client-utils"; -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { ITelemetryBaseLogger } from "@fluidframework/core-interfaces"; import { assert } from "@fluidframework/core-utils/internal"; import { ISequencedDocumentMessage } from "@fluidframework/driver-definitions/internal"; @@ -26,7 +26,7 @@ export class BatchTracker { logger: ITelemetryBaseLogger, batchLengthThreshold: number, batchCountSamplingRate: number, - dateTimeProvider: () => number = () => performance.now(), + dateTimeProvider: () => number = () => performanceNow(), ) { this.logger = createChildLogger({ logger, namespace: "Batching" }); diff --git a/packages/runtime/container-runtime/src/connectionTelemetry.ts b/packages/runtime/container-runtime/src/connectionTelemetry.ts index 5d96deb8749c..10a988bcf120 100644 --- a/packages/runtime/container-runtime/src/connectionTelemetry.ts +++ b/packages/runtime/container-runtime/src/connectionTelemetry.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { IDeltaManagerFull } from "@fluidframework/container-definitions/internal"; import { IContainerRuntimeEvents } from "@fluidframework/container-runtime-definitions/internal"; import type { ITelemetryBaseLogger } from "@fluidframework/core-interfaces"; @@ -92,7 +92,7 @@ class OpPerfTelemetry { private firstConnection = true; private connectionOpSeqNumber: number | undefined; - private readonly bootTime = performance.now(); + private readonly bootTime = performanceNow(); private connectionStartTime = 0; private gap = 0; @@ -204,7 +204,7 @@ class OpPerfTelemetry { if (opsBehind !== undefined) { this.connectionOpSeqNumber = this.deltaManager.lastKnownSeqNumber; this.gap = opsBehind; - this.connectionStartTime = performance.now(); + this.connectionStartTime = performanceNow(); // We might be already up-today. If so, report it right away. if (this.gap <= 0) { @@ -306,7 +306,7 @@ class OpPerfTelemetry { this.connectionOpSeqNumber = undefined; this.logger.sendPerformanceEvent({ eventName: "ConnectionSpeed", - duration: performance.now() - this.connectionStartTime, + duration: performanceNow() - this.connectionStartTime, ops: this.gap, // track time to connect only for first connection. timeToConnect: this.firstConnection diff --git a/packages/runtime/container-runtime/src/deltaScheduler.ts b/packages/runtime/container-runtime/src/deltaScheduler.ts index 0c82717a0f20..b1d8702c9d39 100644 --- a/packages/runtime/container-runtime/src/deltaScheduler.ts +++ b/packages/runtime/container-runtime/src/deltaScheduler.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance, type TypedEventEmitter } from "@fluid-internal/client-utils"; +import { performanceNow, type TypedEventEmitter } from "@fluid-internal/client-utils"; import { IDeltaManagerFull } from "@fluidframework/container-definitions/internal"; import { ISequencedDocumentMessage } from "@fluidframework/driver-definitions/internal"; import type { IContainerRuntimeBaseEvents } from "@fluidframework/runtime-definitions/internal"; @@ -67,7 +67,7 @@ export class DeltaScheduler { private readonly batchBegin = (message: ISequencedDocumentMessage): void => { if (!this.processingStartTime) { - this.processingStartTime = performance.now(); + this.processingStartTime = performanceNow(); } if (this.schedulingLog === undefined && this.schedulingCount % 500 === 0) { // Every 500th time we are scheduling the inbound queue, we log telemetry for the @@ -79,7 +79,7 @@ export class DeltaScheduler { numberOfBatchesProcessed: 0, firstSequenceNumber: message.sequenceNumber, lastSequenceNumber: message.sequenceNumber, - startTime: performance.now(), + startTime: performanceNow(), }; } }; @@ -92,7 +92,7 @@ export class DeltaScheduler { } if (this.shouldRunScheduler()) { - const currentTime = performance.now(); + const currentTime = performanceNow(); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const elapsedTime = currentTime - this.processingStartTime!; if (elapsedTime > this.currentAllowedProcessingTimeForTurn) { @@ -126,7 +126,7 @@ export class DeltaScheduler { processingTime: formatTick(this.schedulingLog.totalProcessingTime), numberOfTurns: this.schedulingLog.numberOfTurns, batchesProcessed: this.schedulingLog.numberOfBatchesProcessed, - timeToResume: formatTick(performance.now() - currentTime), + timeToResume: formatTick(performanceNow() - currentTime), }); } this.deltaManager.inbound.resume(); @@ -141,7 +141,7 @@ export class DeltaScheduler { if (this.schedulingLog) { // Add the time taken for processing the final ops to the total processing time in the // telemetry log object. - const currentTime = performance.now(); + const currentTime = performanceNow(); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion this.schedulingLog.totalProcessingTime += currentTime - this.processingStartTime!; diff --git a/packages/runtime/container-runtime/src/inboundBatchAggregator.ts b/packages/runtime/container-runtime/src/inboundBatchAggregator.ts index c5cc968cde69..c8f6b4ed63af 100644 --- a/packages/runtime/container-runtime/src/inboundBatchAggregator.ts +++ b/packages/runtime/container-runtime/src/inboundBatchAggregator.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { IDeltaManagerFull } from "@fluidframework/container-definitions/internal"; import { assert } from "@fluidframework/core-utils/internal"; import { ISequencedDocumentMessage } from "@fluidframework/driver-definitions/internal"; @@ -227,14 +227,14 @@ export class InboundBatchAggregator { private pauseQueue(): void { assert(!this.localPaused, 0x297 /* "always called from resumed state" */); this.localPaused = true; - this.timePaused = performance.now(); + this.timePaused = performanceNow(); // eslint-disable-next-line @typescript-eslint/no-floating-promises this.deltaManager.inbound.pause(); } private resumeQueue(startBatch: number, messageEndBatch: ISequencedDocumentMessage): void { const endBatch = messageEndBatch.sequenceNumber; - const duration = this.localPaused ? performance.now() - this.timePaused : undefined; + const duration = this.localPaused ? performanceNow() - this.timePaused : undefined; this.batchCount++; if (this.batchCount % 1000 === 1) { diff --git a/packages/utils/telemetry-utils/src/logger.ts b/packages/utils/telemetry-utils/src/logger.ts index a1ad3354ae3d..47cbb22eb5ad 100644 --- a/packages/utils/telemetry-utils/src/logger.ts +++ b/packages/utils/telemetry-utils/src/logger.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import { type ITelemetryBaseEvent, type ITelemetryBaseLogger, @@ -722,11 +722,11 @@ export class PerformanceEvent { } public get duration(): number { - return performance.now() - this.startTime; + return performanceNow() - this.startTime; } private event?: ITelemetryGenericEventExt; - private readonly startTime = performance.now(); + private readonly startTime = performanceNow(); private startMark?: string; protected constructor( diff --git a/packages/utils/telemetry-utils/src/sampledTelemetryHelper.ts b/packages/utils/telemetry-utils/src/sampledTelemetryHelper.ts index ac4c1f986efd..a6206f636410 100644 --- a/packages/utils/telemetry-utils/src/sampledTelemetryHelper.ts +++ b/packages/utils/telemetry-utils/src/sampledTelemetryHelper.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { performance } from "@fluid-internal/client-utils"; +import { performanceNow } from "@fluid-internal/client-utils"; import type { IDisposable, ITelemetryBaseProperties } from "@fluidframework/core-interfaces"; import { assert } from "@fluidframework/core-utils/internal"; @@ -192,9 +192,9 @@ export class SampledTelemetryHelper< codeToMeasure: () => MeasureReturnType, bucket: string = "", ): MeasureReturnType { - const start = performance.now(); + const start = performanceNow(); const returnValue = codeToMeasure(); - const duration = performance.now() - start; + const duration = performanceNow() - start; let loggerData = this.measurementsMap.get(bucket); if (loggerData === undefined) { From 216974ba5b692fd37efc70197b7db412c9d19c85 Mon Sep 17 00:00:00 2001 From: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> Date: Thu, 30 Jan 2025 15:30:40 -0800 Subject: [PATCH 04/28] refactor(container-runtime): Enable `unicorn/no-negated-condition` eslint rule and fix violations (#23696) Part of a multi-PR process to migrate this package off of our deprecated "minimal" eslint config to our "recommended" one. [AB#3027](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/3027) --- .../runtime/container-runtime/.eslintrc.cjs | 4 +-- .../src/blobManager/blobManager.ts | 6 ++-- .../src/channelCollection.ts | 36 +++++++++---------- .../container-runtime/src/containerRuntime.ts | 30 ++++++++-------- .../container-runtime/src/dataStoreContext.ts | 6 ++-- .../src/gc/garbageCollection.ts | 8 ++--- .../container-runtime/src/gc/gcTelemetry.ts | 6 ++-- .../src/pendingStateManager.ts | 10 +++--- .../src/summary/documentSchema.ts | 8 ++--- .../src/summary/orderedClientElection.ts | 20 +++++------ .../src/summary/runningSummarizer.ts | 6 ++-- .../summary/summarizerNode/summarizerNode.ts | 8 ++--- .../src/summary/summaryCollection.ts | 6 ++-- 13 files changed, 76 insertions(+), 78 deletions(-) diff --git a/packages/runtime/container-runtime/.eslintrc.cjs b/packages/runtime/container-runtime/.eslintrc.cjs index 029951a92897..8a7a13bb7223 100644 --- a/packages/runtime/container-runtime/.eslintrc.cjs +++ b/packages/runtime/container-runtime/.eslintrc.cjs @@ -59,6 +59,7 @@ module.exports = { "unicorn/no-array-callback-reference": "error", "unicorn/no-array-for-each": "error", "unicorn/no-lonely-if": "error", + "unicorn/no-negated-condition": "error", "unicorn/no-new-array": "error", "unicorn/no-null": "error", "unicorn/no-zero-fractions": "error", @@ -71,9 +72,6 @@ module.exports = { "unicorn/switch-case-braces": "error", "unicorn/throw-new-error": "error", - // TODO: - // unicorn/no-negated-condition - // #endregion }, overrides: [ diff --git a/packages/runtime/container-runtime/src/blobManager/blobManager.ts b/packages/runtime/container-runtime/src/blobManager/blobManager.ts index 4c3206d2618d..6565da7ec687 100644 --- a/packages/runtime/container-runtime/src/blobManager/blobManager.ts +++ b/packages/runtime/container-runtime/src/blobManager/blobManager.ts @@ -867,10 +867,10 @@ export class BlobManager extends TypedEventEmitter { resolve(); } }; - if (!entry.attached) { - this.on("blobAttached", onBlobAttached); - } else { + if (entry.attached) { resolve(); + } else { + this.on("blobAttached", onBlobAttached); } }), ); diff --git a/packages/runtime/container-runtime/src/channelCollection.ts b/packages/runtime/container-runtime/src/channelCollection.ts index 80444810e660..c01ec34895fb 100644 --- a/packages/runtime/container-runtime/src/channelCollection.ts +++ b/packages/runtime/container-runtime/src/channelCollection.ts @@ -324,41 +324,41 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable { unreferencedDataStoreCount++; } // If we have a detached container, then create local data store contexts. - if (this.parentContext.attachState !== AttachState.Detached) { - let snapshotForRemoteFluidDatastoreContext: ISnapshot | ISnapshotTree = value; - if (isInstanceOfISnapshot(baseSnapshot)) { - snapshotForRemoteFluidDatastoreContext = { - ...baseSnapshot, - snapshotTree: value, - }; + if (this.parentContext.attachState === AttachState.Detached) { + if (typeof value !== "object") { + throw new LoggingError("Snapshot should be there to load from!!"); } - dataStoreContext = new RemoteFluidDataStoreContext({ + const snapshotTree = value; + dataStoreContext = new LocalFluidDataStoreContext({ id: key, - snapshot: snapshotForRemoteFluidDatastoreContext, + pkg: undefined, parentContext: this.wrapContextForInnerChannel(key), storage: this.parentContext.storage, scope: this.parentContext.scope, createSummarizerNodeFn: this.parentContext.getCreateChildSummarizerNodeFn(key, { type: CreateSummarizerNodeSource.FromSummary, }), - loadingGroupId: value.groupId, + makeLocallyVisibleFn: () => this.makeDataStoreLocallyVisible(key), + snapshotTree, }); } else { - if (typeof value !== "object") { - throw new LoggingError("Snapshot should be there to load from!!"); + let snapshotForRemoteFluidDatastoreContext: ISnapshot | ISnapshotTree = value; + if (isInstanceOfISnapshot(baseSnapshot)) { + snapshotForRemoteFluidDatastoreContext = { + ...baseSnapshot, + snapshotTree: value, + }; } - const snapshotTree = value; - dataStoreContext = new LocalFluidDataStoreContext({ + dataStoreContext = new RemoteFluidDataStoreContext({ id: key, - pkg: undefined, + snapshot: snapshotForRemoteFluidDatastoreContext, parentContext: this.wrapContextForInnerChannel(key), storage: this.parentContext.storage, scope: this.parentContext.scope, createSummarizerNodeFn: this.parentContext.getCreateChildSummarizerNodeFn(key, { type: CreateSummarizerNodeSource.FromSummary, }), - makeLocallyVisibleFn: () => this.makeDataStoreLocallyVisible(key), - snapshotTree, + loadingGroupId: value.groupId, }); } this.contexts.addBoundOrRemoted(dataStoreContext); @@ -1063,7 +1063,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable { } const idToLog = - originalRequest !== undefined ? urlToGCNodePath(originalRequest.url) : dataStoreNodePath; + originalRequest === undefined ? dataStoreNodePath : urlToGCNodePath(originalRequest.url); // Log the package details asynchronously since getInitialSnapshotDetails is async const recentlyDeletedContext = this.contexts.getRecentlyDeletedContext(id); diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index c0c0f75ea6a7..00c60e4592e6 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -803,9 +803,7 @@ async function createSummarizer(loader: ILoader, url: string): Promise { // If the GC state hasn't been initialized yet, initialize it and return. - if (!initialized) { - await this.initializeGCStateFromBaseSnapshotP; - } else { + if (initialized) { // If the GC state has been initialized, update the tracking of unreferenced nodes as per the current // reference timestamp. for (const [, nodeStateTracker] of this.unreferencedNodesState) { nodeStateTracker.updateTracking(currentReferenceTimestampMs); } + } else { + await this.initializeGCStateFromBaseSnapshotP; } event.end({ details: { initialized, unrefNodeCount: this.unreferencedNodesState.size }, @@ -1034,7 +1034,7 @@ export class GarbageCollector implements IGarbageCollector { // trackedId will be either DataStore or Blob ID (not sub-DataStore ID, since some of those are unrecognized by GC) const trackedId = node.path; const isTombstoned = this.tombstones.includes(trackedId); - const fullPath = request !== undefined ? urlToGCNodePath(request.url) : trackedId; + const fullPath = request === undefined ? trackedId : urlToGCNodePath(request.url); // This will log if appropriate this.telemetryTracker.nodeUsed(trackedId, { diff --git a/packages/runtime/container-runtime/src/gc/gcTelemetry.ts b/packages/runtime/container-runtime/src/gc/gcTelemetry.ts index 0b4087ad1df8..f85f256488b1 100644 --- a/packages/runtime/container-runtime/src/gc/gcTelemetry.ts +++ b/packages/runtime/container-runtime/src/gc/gcTelemetry.ts @@ -230,9 +230,9 @@ export class GCTelemetryTracker { type: nodeType, unrefTime: nodeStateTracker?.unreferencedTimestampMs ?? -1, age: - nodeStateTracker !== undefined - ? currentReferenceTimestampMs - nodeStateTracker.unreferencedTimestampMs - : -1, + nodeStateTracker === undefined + ? -1 + : currentReferenceTimestampMs - nodeStateTracker.unreferencedTimestampMs, timeout, isTombstoned, ...tagCodeArtifacts({ id: untaggedId, fromId: untaggedFromId }), diff --git a/packages/runtime/container-runtime/src/pendingStateManager.ts b/packages/runtime/container-runtime/src/pendingStateManager.ts index e2b689892bd9..e466bc68c297 100644 --- a/packages/runtime/container-runtime/src/pendingStateManager.ts +++ b/packages/runtime/container-runtime/src/pendingStateManager.ts @@ -357,15 +357,15 @@ export class PendingStateManager implements IDisposable { } // applyStashedOp will cause the DDS to behave as if it has sent the op but not actually send it const localOpMetadata = await this.stateHandler.applyStashedOp(nextMessage.content); - if (!this.stateHandler.isAttached()) { - if (localOpMetadata !== undefined) { - throw new Error("Local Op Metadata must be undefined when not attached"); - } - } else { + if (this.stateHandler.isAttached()) { nextMessage.localOpMetadata = localOpMetadata; // then we push onto pendingMessages which will cause PendingStateManager to resubmit when we connect patchbatchInfo(nextMessage); // Back compat this.pendingMessages.push(nextMessage); + } else { + if (localOpMetadata !== undefined) { + throw new Error("Local Op Metadata must be undefined when not attached"); + } } } catch (error) { throw DataProcessingError.wrapIfUnrecognized(error, "applyStashedOp", nextMessage); diff --git a/packages/runtime/container-runtime/src/summary/documentSchema.ts b/packages/runtime/container-runtime/src/summary/documentSchema.ts index 342faed3fb16..2eda7e9db93e 100644 --- a/packages/runtime/container-runtime/src/summary/documentSchema.ts +++ b/packages/runtime/container-runtime/src/summary/documentSchema.ts @@ -503,9 +503,8 @@ export class DocumentsSchemaController { // Schema coming from document metadata (snapshot we loaded from), or if no document exists // (this is a new document) then this is the same as desiredSchema (same as session schema in such case). // Latter is importnat sure that's what will go into summary. - this.documentSchema = !existing - ? this.desiredSchema - : ((documentMetadataSchema as IDocumentSchemaCurrent) ?? + this.documentSchema = existing + ? ((documentMetadataSchema as IDocumentSchemaCurrent) ?? ({ version: currentDocumentVersionSchema, // see comment in summarizeDocumentSchema() on why it has to stay zero @@ -515,7 +514,8 @@ export class DocumentsSchemaController { runtime: { explicitSchemaControl: boolToProp(!existing && features.explicitSchemaControl), }, - } satisfies IDocumentSchemaCurrent)); + } satisfies IDocumentSchemaCurrent)) + : this.desiredSchema; checkRuntimeCompatibility(this.documentSchema, "document"); this.validateSeqNumber(this.documentSchema.refSeq, snapshotSequenceNumber, "summary"); diff --git a/packages/runtime/container-runtime/src/summary/orderedClientElection.ts b/packages/runtime/container-runtime/src/summary/orderedClientElection.ts index 523a02b47088..07f48760a4fd 100644 --- a/packages/runtime/container-runtime/src/summary/orderedClientElection.ts +++ b/packages/runtime/container-runtime/src/summary/orderedClientElection.ts @@ -604,8 +604,15 @@ export class OrderedClientElection this._eligibleCount--; if (this._electedClient === client) { // Removing the _electedClient. There are 2 possible cases: - if (this._electedParent !== client) { - // 1. The _electedClient is a summarizer that we've been allowing to finish its work. + if (this._electedParent === client) { + // 1. The _electedClient is an interactive client that has left the quorum. + // Automatically shift to next oldest client. + const nextClient = + this.findFirstEligibleParent(this._electedParent?.youngerClient) ?? + this.findFirstEligibleParent(this.orderedClientCollection.oldestClient); + this.tryElectingClient(nextClient, sequenceNumber, "RemoveClient"); + } else { + // 2. The _electedClient is a summarizer that we've been allowing to finish its work. // Let the _electedParent become the _electedClient so that it can start its own summarizer. if (this._electedClient.client.details.type !== summarizerClientType) { throw new UsageError("Elected client should be a summarizer client 1"); @@ -615,13 +622,6 @@ export class OrderedClientElection sequenceNumber, "RemoveSummarizerClient", ); - } else { - // 2. The _electedClient is an interactive client that has left the quorum. - // Automatically shift to next oldest client. - const nextClient = - this.findFirstEligibleParent(this._electedParent?.youngerClient) ?? - this.findFirstEligibleParent(this.orderedClientCollection.oldestClient); - this.tryElectingClient(nextClient, sequenceNumber, "RemoveClient"); } } else if (this._electedParent === client) { // Removing the _electedParent (but not _electedClient). @@ -690,7 +690,7 @@ export class OrderedClientElection sequenceNumber, electedClientId: this.electedClient?.clientId, electedParentId: this.electedParent?.clientId, - isEligible: client !== undefined ? this.isEligibleFn(client) : false, + isEligible: client === undefined ? false : this.isEligibleFn(client), isSummarizerClient: client?.client.details.type === summarizerClientType, reason, }); diff --git a/packages/runtime/container-runtime/src/summary/runningSummarizer.ts b/packages/runtime/container-runtime/src/summary/runningSummarizer.ts index 80b9d393382c..176cd874748c 100644 --- a/packages/runtime/container-runtime/src/summary/runningSummarizer.ts +++ b/packages/runtime/container-runtime/src/summary/runningSummarizer.ts @@ -813,9 +813,9 @@ export class RunningSummarizer // If submit summary failed, use maxAttemptsForSubmitFailures. Else use the defaultMaxAttempts. // Note: Check "summarySubmitted" result first because if it fails, ack nack would fail as well. const submitSummaryResult = await results.summarySubmitted; - maxAttempts = !submitSummaryResult.success - ? this.maxAttemptsForSubmitFailures - : defaultMaxAttempts; + maxAttempts = submitSummaryResult.success + ? defaultMaxAttempts + : this.maxAttemptsForSubmitFailures; // Emit "summarize" event for this failed attempt. status = "failure"; diff --git a/packages/runtime/container-runtime/src/summary/summarizerNode/summarizerNode.ts b/packages/runtime/container-runtime/src/summary/summarizerNode/summarizerNode.ts index 415fc2789128..f6a12cc16f4c 100644 --- a/packages/runtime/container-runtime/src/summary/summarizerNode/summarizerNode.ts +++ b/packages/runtime/container-runtime/src/summary/summarizerNode/summarizerNode.ts @@ -224,14 +224,14 @@ export class SummarizerNode implements IRootSummarizerNode { 0x5df /* Summarize should not be called when not tracking the summary */, ); incrementalSummaryContext = - this._lastSummaryReferenceSequenceNumber !== undefined - ? { + this._lastSummaryReferenceSequenceNumber === undefined + ? undefined + : { summarySequenceNumber: this.wipReferenceSequenceNumber, latestSummarySequenceNumber: this._lastSummaryReferenceSequenceNumber, // TODO: remove summaryPath. summaryPath: this.summaryHandleId, - } - : undefined; + }; } const result = await this.summarizeInternalFn( diff --git a/packages/runtime/container-runtime/src/summary/summaryCollection.ts b/packages/runtime/container-runtime/src/summary/summaryCollection.ts index 242edc9b7bd1..3c01edbcb413 100644 --- a/packages/runtime/container-runtime/src/summary/summaryCollection.ts +++ b/packages/runtime/container-runtime/src/summary/summaryCollection.ts @@ -385,10 +385,10 @@ export class SummaryCollection extends TypedEventEmitter Date: Thu, 30 Jan 2025 16:06:08 -0800 Subject: [PATCH 05/28] fix(docs): Don't display `Alpha` alert for `Legacy` API items (#23704) We have been displaying dual "Alpha" and "Legacy" alerts for items tagged with both `@alpha` and `@legacy`, but this is incorrect. `@alpha` + `@legacy` should map to _just_ "Legacy". API items' details sections already _only_ displayed legacy API notices for these cases, but table entries displayed both alerts, which was misleading. Example before: ![image](https://github.com/user-attachments/assets/163a2ea8-35e2-40dc-9d3d-8dd2035b06b5) Example after: ![image](https://github.com/user-attachments/assets/216b6ef6-946e-4150-9b17-81fbb3e61183) --- .../render-api-documentation.mjs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/docs/infra/api-markdown-documenter/render-api-documentation.mjs b/docs/infra/api-markdown-documenter/render-api-documentation.mjs index a2a6019c223c..92d015942225 100644 --- a/docs/infra/api-markdown-documenter/render-api-documentation.mjs +++ b/docs/infra/api-markdown-documenter/render-api-documentation.mjs @@ -128,15 +128,18 @@ export async function renderApiDocumentation(inputDir, outputDir, uriRootDir, ap if (ApiItemUtilities.isDeprecated(apiItem)) { alerts.push("Deprecated"); } + + // If an item is `@legacy`, ignore its release tag (we use `@alpha`+`@legacy` to mean something + // entirely different from `@alpha`, so displaying the release tag would be misleading). if (ApiItemUtilities.hasModifierTag(apiItem, "@legacy")) { alerts.push("Legacy"); - } - - const releaseTag = ApiItemUtilities.getEffectiveReleaseLevel(apiItem); - if (releaseTag === ReleaseTag.Alpha) { - alerts.push("Alpha"); - } else if (releaseTag === ReleaseTag.Beta) { - alerts.push("Beta"); + } else { + const releaseTag = ApiItemUtilities.getEffectiveReleaseLevel(apiItem); + if (releaseTag === ReleaseTag.Alpha) { + alerts.push("Alpha"); + } else if (releaseTag === ReleaseTag.Beta) { + alerts.push("Beta"); + } } } return alerts; From f67994577597aae6dc8b42f3c6557c744adc0964 Mon Sep 17 00:00:00 2001 From: Jason Hartman Date: Thu, 30 Jan 2025 16:40:15 -0800 Subject: [PATCH 06/28] improvement(client-azure-client): deprecate legacy exports (#23703) and replace internal uses. Note: legacy.public API appears changed, but that is because current FF configuration for api-extractor (without `bundlePackages`) doesn't have direct re-exports correctly reported. (This is largely moot as all legacy.* exports are exposed from the same /legacy path.) Clean up `AzureUser` use in end to end tests where test data type is not really `AzureUser`, but an object with `id` and `name`. --- .changeset/quick-chairs-smile.md | 8 ++++++ .../azure-client.legacy.alpha.api.md | 9 ++++-- .../azure-client.legacy.public.api.md | 4 --- .../service-clients/azure-client/src/index.ts | 28 ++++++++++++++++++- .../azure-client/.eslintrc.cjs | 10 +++++++ .../src/test/AzureClientFactory.ts | 5 +++- .../src/test/AzureTokenFactory.ts | 2 +- .../azure-client/src/test/audience.spec.ts | 2 +- .../azure-client/src/test/signals.spec.ts | 21 ++++++++------ 9 files changed, 71 insertions(+), 18 deletions(-) create mode 100644 .changeset/quick-chairs-smile.md diff --git a/.changeset/quick-chairs-smile.md b/.changeset/quick-chairs-smile.md new file mode 100644 index 000000000000..c79b36d0ba52 --- /dev/null +++ b/.changeset/quick-chairs-smile.md @@ -0,0 +1,8 @@ +--- +"@fluidframework/azure-client": minor +--- +--- +"section": deprecation +--- + +[Deprecate azure-client re-exported legacy APIs](https://github.com/microsoft/FluidFramework/issues/23702) including `ITokenClaims` and `ScopeType`. See [issue #23702](https://github.com/microsoft/FluidFramework/issues/23702) for details and alternatives. diff --git a/packages/service-clients/azure-client/api-report/azure-client.legacy.alpha.api.md b/packages/service-clients/azure-client/api-report/azure-client.legacy.alpha.api.md index d749f5953051..67c320162fbf 100644 --- a/packages/service-clients/azure-client/api-report/azure-client.legacy.alpha.api.md +++ b/packages/service-clients/azure-client/api-report/azure-client.legacy.alpha.api.md @@ -82,7 +82,8 @@ export { ITelemetryBaseEvent } export { ITelemetryBaseLogger } -export { ITokenClaims } +// @alpha @deprecated +export type ITokenClaims = ITokenClaims_2; export { ITokenProvider } @@ -90,6 +91,10 @@ export { ITokenResponse } export { IUser } -export { ScopeType } +// @alpha @deprecated +export const ScopeType: typeof ScopeType_2; + +// @alpha @deprecated +export type ScopeType = ScopeType_2; ``` diff --git a/packages/service-clients/azure-client/api-report/azure-client.legacy.public.api.md b/packages/service-clients/azure-client/api-report/azure-client.legacy.public.api.md index a1e15d6545db..0d1d94ca21c2 100644 --- a/packages/service-clients/azure-client/api-report/azure-client.legacy.public.api.md +++ b/packages/service-clients/azure-client/api-report/azure-client.legacy.public.api.md @@ -82,14 +82,10 @@ export { ITelemetryBaseEvent } export { ITelemetryBaseLogger } -export { ITokenClaims } - export { ITokenProvider } export { ITokenResponse } export { IUser } -export { ScopeType } - ``` diff --git a/packages/service-clients/azure-client/src/index.ts b/packages/service-clients/azure-client/src/index.ts index 3f289c0f3508..8785835dc631 100644 --- a/packages/service-clients/azure-client/src/index.ts +++ b/packages/service-clients/azure-client/src/index.ts @@ -26,7 +26,33 @@ export type { export type { ITokenProvider, ITokenResponse } from "@fluidframework/routerlicious-driver"; export type { IUser } from "@fluidframework/driver-definitions"; -export { type ITokenClaims, ScopeType } from "@fluidframework/driver-definitions/internal"; +import { + type ITokenClaims as ITokenClaimsBase, + ScopeType as ScopeTypeBase, +} from "@fluidframework/driver-definitions/internal"; + +/** + * {@inheritdoc @fluidframework/driver-definitions/legacy#ITokenClaims} + * @legacy + * @alpha + * @deprecated Consider importing from `@fluidframework/driver-definitions/legacy` - to be removed in 2.40 + */ +export type ITokenClaims = ITokenClaimsBase; + +/** + * {@inheritdoc @fluidframework/driver-definitions/legacy#ScopeType} + * @legacy + * @alpha + * @deprecated Use ScopeType from \@fluidframework/driver-definitions/legacy - to be removed in 2.40 + */ +export const ScopeType = ScopeTypeBase; +/** + * {@inheritdoc @fluidframework/driver-definitions/legacy#ScopeType} + * @legacy + * @alpha + * @deprecated Use ScopeType from \@fluidframework/driver-definitions/legacy - to be removed in 2.40 + */ +export type ScopeType = ScopeTypeBase; // Re-export so developers can build loggers without pulling in core-interfaces export type { diff --git a/packages/service-clients/end-to-end-tests/azure-client/.eslintrc.cjs b/packages/service-clients/end-to-end-tests/azure-client/.eslintrc.cjs index f3531ca7789e..594c3304b748 100644 --- a/packages/service-clients/end-to-end-tests/azure-client/.eslintrc.cjs +++ b/packages/service-clients/end-to-end-tests/azure-client/.eslintrc.cjs @@ -10,6 +10,16 @@ module.exports = { "@typescript-eslint/strict-boolean-expressions": "off", // requires strictNullChecks=true in tsconfig "@fluid-internal/fluid/no-unchecked-record-access": "warn", }, + overrides: [ + { + // Rules only for test files + files: ["*.spec.ts", "*.test.ts", "**/test/**"], + rules: { + // Some deprecated APIs are permissible in tests; use `warn` to keep them visible + "import/no-deprecated": "warn", + }, + }, + ], parserOptions: { project: ["./src/test/tsconfig.json"], }, diff --git a/packages/service-clients/end-to-end-tests/azure-client/src/test/AzureClientFactory.ts b/packages/service-clients/end-to-end-tests/azure-client/src/test/AzureClientFactory.ts index d5a2d39dbc7e..7adacfecc33b 100644 --- a/packages/service-clients/end-to-end-tests/azure-client/src/test/AzureClientFactory.ts +++ b/packages/service-clients/end-to-end-tests/azure-client/src/test/AzureClientFactory.ts @@ -9,7 +9,6 @@ import { AzureRemoteConnectionConfig, ITelemetryBaseLogger, } from "@fluidframework/azure-client"; -import { type ScopeType } from "@fluidframework/azure-client/internal"; import { AzureClient as AzureClientLegacy, AzureLocalConnectionConfig as AzureLocalConnectionConfigLegacy, @@ -17,6 +16,7 @@ import { ITelemetryBaseLogger as ITelemetryBaseLoggerLegacy, } from "@fluidframework/azure-client-legacy"; import { IConfigProviderBase } from "@fluidframework/core-interfaces"; +import { ScopeType } from "@fluidframework/driver-definitions/internal"; import { MockLogger, createChildLogger, @@ -28,6 +28,9 @@ import { v4 as uuid } from "uuid"; import { createAzureTokenProvider } from "./AzureTokenFactory.js"; +// eslint-disable-next-line unicorn/prefer-export-from +export { ScopeType }; + /** * This function will determine if local or remote mode is required (based on FLUID_CLIENT), and return a new * {@link AzureClient} instance based on the mode by setting the Connection config accordingly. diff --git a/packages/service-clients/end-to-end-tests/azure-client/src/test/AzureTokenFactory.ts b/packages/service-clients/end-to-end-tests/azure-client/src/test/AzureTokenFactory.ts index 468fdd7fd5dd..fd95b688561e 100644 --- a/packages/service-clients/end-to-end-tests/azure-client/src/test/AzureTokenFactory.ts +++ b/packages/service-clients/end-to-end-tests/azure-client/src/test/AzureTokenFactory.ts @@ -4,7 +4,7 @@ */ import { ITokenProvider } from "@fluidframework/azure-client"; -import { type ScopeType } from "@fluidframework/azure-client/internal"; +import type { ScopeType } from "@fluidframework/driver-definitions/internal"; import { InsecureTokenProvider } from "@fluidframework/test-runtime-utils/internal"; export function createAzureTokenProvider( diff --git a/packages/service-clients/end-to-end-tests/azure-client/src/test/audience.spec.ts b/packages/service-clients/end-to-end-tests/azure-client/src/test/audience.spec.ts index c8705823f8eb..3a40c358bf26 100644 --- a/packages/service-clients/end-to-end-tests/azure-client/src/test/audience.spec.ts +++ b/packages/service-clients/end-to-end-tests/azure-client/src/test/audience.spec.ts @@ -6,7 +6,6 @@ import { strict as assert } from "node:assert"; import { AzureClient, type AzureContainerServices } from "@fluidframework/azure-client"; -import { ScopeType } from "@fluidframework/azure-client/internal"; import { AttachState } from "@fluidframework/container-definitions"; import { ConnectionState } from "@fluidframework/container-loader"; import { ContainerSchema, type IFluidContainer } from "@fluidframework/fluid-static"; @@ -18,6 +17,7 @@ import { createAzureClient, createContainerFromPayload, getContainerIdFromPayloadResponse, + ScopeType, } from "./AzureClientFactory.js"; import * as ephemeralSummaryTrees from "./ephemeralSummaryTrees.js"; import { configProvider, waitForMember, getTestMatrix } from "./utils.js"; diff --git a/packages/service-clients/end-to-end-tests/azure-client/src/test/signals.spec.ts b/packages/service-clients/end-to-end-tests/azure-client/src/test/signals.spec.ts index 32d4d94bd1f9..7f1f3e72b08f 100644 --- a/packages/service-clients/end-to-end-tests/azure-client/src/test/signals.spec.ts +++ b/packages/service-clients/end-to-end-tests/azure-client/src/test/signals.spec.ts @@ -6,7 +6,6 @@ import { strict as assert } from "node:assert"; import { AzureClient, type AzureContainerServices } from "@fluidframework/azure-client"; -import { type AzureUser, ScopeType } from "@fluidframework/azure-client/internal"; import { AttachState } from "@fluidframework/container-definitions"; import { ConnectionState } from "@fluidframework/container-loader"; import { type ContainerSchema, type IFluidContainer } from "@fluidframework/fluid-static"; @@ -17,11 +16,17 @@ import { createAzureClient, createContainerFromPayload, getContainerIdFromPayloadResponse, + ScopeType, } from "./AzureClientFactory.js"; import { SignalerTestDataObject } from "./TestDataObject.js"; import * as ephemeralSummaryTrees from "./ephemeralSummaryTrees.js"; import { configProvider, getTestMatrix } from "./utils.js"; +interface UserIdAndName { + readonly id: string; + readonly name: string; +} + async function createSignalListenerPromise( signaler: SignalerTestDataObject, signalType: string, @@ -54,18 +59,18 @@ for (const testOpts of testMatrix) { const connectedContainers: IFluidContainer[] = []; const connectTimeoutMs = 10_000; const isEphemeral: boolean = testOpts.options.isEphemeral; - const user1: AzureUser = { + const user1 = { id: "test-user-id-1", name: "test-user-name-2", - }; - const user2: AzureUser = { + } as const satisfies UserIdAndName; + const user2 = { id: "test-user-id-1", name: "test-user-name-2", - }; - const user3: AzureUser = { + } as const satisfies UserIdAndName; + const user3 = { id: "test-user-id-1", name: "test-user-name-2", - }; + } as const satisfies UserIdAndName; afterEach(async () => { for (const container of connectedContainers) { @@ -77,7 +82,7 @@ for (const testOpts of testMatrix) { const getOrCreateSignalerContainer = async ( id: string | undefined, - user: AzureUser, + user: UserIdAndName, config?: ReturnType, scopes?: ScopeType[], ): Promise<{ From f6355b0af54489ac371ff6295c8fb006c159b910 Mon Sep 17 00:00:00 2001 From: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:55:55 -0800 Subject: [PATCH 07/28] fix(docs): Provide correct property to `api-markdown-documenter` and fix lingering issues (#23706) Our custom page layout for API documentation was being completely ignored, because we were passing it under an incorrect property name when calling api-markdown-documenter. Adds `//ts-check` for extra validation going forward. Example before (API is tagged as `@legacy`, so it _should_ have a legacy import notice, not an alpha API warning): ![image](https://github.com/user-attachments/assets/0053aca6-9e15-4dd3-ab49-65432ed18ced) Example after: ![image](https://github.com/user-attachments/assets/36caf618-b163-4884-b210-828d90bbc1a6) --- .../api-documentation-layout.mjs | 41 ++++++++++++++----- .../render-api-documentation.mjs | 2 +- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/docs/infra/api-markdown-documenter/api-documentation-layout.mjs b/docs/infra/api-markdown-documenter/api-documentation-layout.mjs index 965fba9a8fdb..c6494dd597fa 100644 --- a/docs/infra/api-markdown-documenter/api-documentation-layout.mjs +++ b/docs/infra/api-markdown-documenter/api-documentation-layout.mjs @@ -3,7 +3,10 @@ * Licensed under the MIT License. */ +//@ts-check + import { + ApiItemKind, ApiItemUtilities, CodeSpanNode, HeadingNode, @@ -41,10 +44,20 @@ const supportDocsLinkSpan = new SpanNode([ * @privateRemarks * If we later wish to differentiate between release tags of `@legacy` items, this function will need * to be updated. + * + * @param apiItem {import("@fluid-tools/api-markdown-documenter").ApiItem} - The API item for which the import notice is being created. */ function createImportNotice(apiItem) { - const packageName = apiItem.getAssociatedPackage().displayName; + const containingPackage = apiItem.getAssociatedPackage(); + if (containingPackage === undefined) { + throw new Error("API item does not have an associated package."); + } + const packageName = containingPackage.displayName; + /** + * @param {string} importSubpath - Subpath beneath the item's package through which the item can be imported. + * @param {string} admonitionTitle - Title to display for the admonition. + */ function createImportAdmonition(importSubpath, admonitionTitle) { return new AdmonitionNode( [ @@ -68,16 +81,16 @@ function createImportNotice(apiItem) { ); } - const releaseTag = ApiItemUtilities.getReleaseTag(apiItem); + const releaseLevel = ApiItemUtilities.getEffectiveReleaseLevel(apiItem); - if (releaseTag === ReleaseTag.Alpha) { + if (releaseLevel === ReleaseTag.Alpha) { return createImportAdmonition( "alpha", "This API is provided as an alpha preview and may change without notice.", ); } - if (releaseTag === ReleaseTag.Beta) { + if (releaseLevel === ReleaseTag.Beta) { return createImportAdmonition( "beta", "This API is provided as a beta preview and may change without notice.", @@ -91,6 +104,8 @@ function createImportNotice(apiItem) { * Creates a special use notice for the provided API item, if one is appropriate. * * If the item is tagged as "@system", displays an internal notice with use notes. + * + * @param apiItem {import("@fluid-tools/api-markdown-documenter").ApiItem} - The API item for which the system notice is being created. */ function createSystemNotice(apiItem) { if (ApiItemUtilities.ancestryHasModifierTag(apiItem, "@system")) { @@ -129,13 +144,17 @@ function createSystemNotice(apiItem) { * * 1. See (if any) * - * @param {@microsoft/api-extractor-model#ApiItem} apiItem - The API item being rendered. - * @param {@fluid-tools/api-markdown-documenter#SectionNode[] | undefined} itemSpecificContent - API item-specific details to be included in the default layout. - * @param {@fluid-tools/api-markdown-documenter#ApiItemTransformationConfiguration} config - Transformation configuration. + * @param {import("@fluid-tools/api-markdown-documenter").ApiItem} apiItem - The API item being rendered. + * @param {import("@fluid-tools/api-markdown-documenter").SectionNode[] | undefined} itemSpecificContent - API item-specific details to be included in the default layout. + * @param {import("@fluid-tools/api-markdown-documenter").ApiItemTransformationConfiguration} config - Transformation configuration. * * @returns An array of sections describing the layout. See {@link @fluid-tools/api-markdown-documenter#ApiItemTransformationConfiguration.createDefaultLayout}. */ export function layoutContent(apiItem, itemSpecificContent, config) { + if (apiItem.kind === ApiItemKind.None) { + throw new Error("Invalid API item kind."); + } + const sections = []; // Render summary comment (if any) @@ -144,7 +163,7 @@ export function layoutContent(apiItem, itemSpecificContent, config) { sections.push(new SectionNode([summary])); } - // Render system notice (if any) that supercedes deprecation and import notices + // Render system notice (if any) that supersedes deprecation and import notices const systemNotice = createSystemNotice(apiItem); if (systemNotice !== undefined) { sections.push(new SectionNode([systemNotice])); @@ -208,7 +227,7 @@ export function layoutContent(apiItem, itemSpecificContent, config) { // Add heading to top of section only if this is being rendered to a parent item. // Document items have their headings handled specially. - return ApiItemUtilities.doesItemRequireOwnDocument(apiItem, config.documentBoundaries) + return ["Document", "Folder"].includes(config.hierarchy[apiItem.kind].kind) ? sections : [ new SectionNode( @@ -226,8 +245,8 @@ export function layoutContent(apiItem, itemSpecificContent, config) { * * @remarks Displayed as a Docusaurus admonition. See {@link AdmonitionNode} and {@link renderAdmonitionNode}. * - * @param {@microsoft/api-extractor-model#ApiItem} apiItem - The API item being rendered. - * @param {@fluid-tools/api-markdown-documenter#ApiItemTransformationConfiguration} config - Transformation configuration. + * @param {import("@fluid-tools/api-markdown-documenter").ApiItem} apiItem - The API item being rendered. + * @param {import("@fluid-tools/api-markdown-documenter").ApiItemTransformationConfiguration} config - Transformation configuration. * * @returns The doc section if the API item had a `@remarks` comment, otherwise `undefined`. */ diff --git a/docs/infra/api-markdown-documenter/render-api-documentation.mjs b/docs/infra/api-markdown-documenter/render-api-documentation.mjs index 92d015942225..23099f0a8af0 100644 --- a/docs/infra/api-markdown-documenter/render-api-documentation.mjs +++ b/docs/infra/api-markdown-documenter/render-api-documentation.mjs @@ -119,7 +119,7 @@ export async function renderApiDocumentation(inputDir, outputDir, uriRootDir, ap uriRoot: uriRootDir, includeBreadcrumb: false, // Docusaurus includes this by default based on file hierarchy includeTopLevelDocumentHeading: false, // We inject `title` front-matter metadata instead - createDefaultLayout: layoutContent, + defaultSectionLayout: layoutContent, getAlertsForItem: (apiItem) => { const alerts = []; if (ApiItemUtilities.hasModifierTag(apiItem, "@system")) { From 17372d0beb3326a0cc9a41a1bf4d1e57143bb65f Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Thu, 30 Jan 2025 21:08:54 -0800 Subject: [PATCH 08/28] improvement(build-cli): Remove processing of lerna.json files (#22969) We have long ago stopped using lerna or lerna.json files, but there is a code path that expects them in the build tools. That code has been removed. Once these changes are released, lerna.json files can be safely removed from release groups using the latest build-tools. --- .github/labeler-areas.yml | 1 - README.md | 8 +++---- .../packages/build-cli/docs/generate.md | 9 ++++---- .../src/commands/generate/buildVersion.ts | 22 ++++++------------- .../packages/build-cli/src/library/package.ts | 16 +++----------- build-tools/packages/build-tools/README.md | 2 +- common/build/build-common/prettier.config.cjs | 7 ------ prettier.config.cjs | 7 ------ scripts/update-package-version.sh | 2 +- .../templates/build-docker-service.yml | 2 +- .../templates/build-npm-client-package.yml | 2 +- .../pipelines/templates/build-npm-package.yml | 2 +- .../templates/include-set-package-version.yml | 2 +- 13 files changed, 24 insertions(+), 58 deletions(-) diff --git a/.github/labeler-areas.yml b/.github/labeler-areas.yml index bc367fa16578..98f2f825285b 100644 --- a/.github/labeler-areas.yml +++ b/.github/labeler-areas.yml @@ -81,7 +81,6 @@ - server/routerlicious/.changeset/** dependencies: - - lerna-package-lock.json - package-lock.json - pnpm-lock.yaml diff --git a/README.md b/README.md index 1013f3eb2207..e2cf4c45da10 100644 --- a/README.md +++ b/README.md @@ -52,13 +52,13 @@ Here's the list of release group workspaces: - [./experimental](./experimental) (Published in the `@fluid-experimental/` namespace) - [./examples](./examples) (Not published, live in the `@fluid-example/` namespace) - [./azure](./azure). (Published in the `@fluidframework/` namespace) -- routerlicious (Reference Fluid Ordering Service) (Rooted in [./server/routerlicious](./server/routerlicious). Configured by [./server/routerlicious/lerna.json](server/routerlicious/lerna.json)) +- routerlicious (Reference Fluid Ordering Service) (Rooted in [./server/routerlicious](./server/routerlicious). Configured by [./server/routerlicious/pnpm-workspace.yaml](server/routerlicious/pnpm-workspace.yaml)) - [Packages](./server/routerlicious/packages) (Published in the `@fluidframework/` namespace) -- gitrest (Rooted in [./server/gitrest](./server/gitrest). Configured by [./server/gitrest/lerna.json](./server/gitrest/lerna.json)) +- gitrest (Rooted in [./server/gitrest](./server/gitrest). Configured by [./server/gitrest/pnpm-workspace.yaml](./server/gitrest/pnpm-workspace.yaml)) - [Packages](./server/gitrest/packages) (Published in the `@fluidframework/` namespace) -- historian (Rooted in [./server/historian](./server/historian). Configured by [./server/historian/lerna.json](./server/historian/lerna.json)) +- historian (Rooted in [./server/historian](./server/historian). Configured by [./server/historian/pnpm-workspace.yaml](./server/historian/pnpm-workspace.yaml)) - [Packages](./server/historian/packages) (Published in the `@fluidframework/` namespace) -- build-tools (Rooted in [./build-tools](./build-tools). Configured by [./build-tools/lerna.json](./build-tools/lerna.json)) +- build-tools (Rooted in [./build-tools](./build-tools). Configured by [./build-tools/pnpm-workspace.yaml](./build-tools/pnpm-workspace.yaml)) - [Packages](./build-tools/packages) (Published in a mix of `@fluidframework/` and `@fluid-tools/` namespaces) Here's a list of other sets of other packages (each package within these groups is versioned independently, diff --git a/build-tools/packages/build-cli/docs/generate.md b/build-tools/packages/build-cli/docs/generate.md index 09575d915fee..618dd547b4df 100644 --- a/build-tools/packages/build-cli/docs/generate.md +++ b/build-tools/packages/build-cli/docs/generate.md @@ -77,7 +77,7 @@ _See code: [src/commands/generate/assertTags.ts](https://github.com/microsoft/Fl ## `flub generate buildVersion` -This command is used to compute the version number of Fluid packages. The release version number is based on what's in the lerna.json/package.json. The CI pipeline will supply the build number and branch to determine the prerelease suffix if it is not a tagged build +This command is used to compute the version number of Fluid packages. The release version number is based on what's in the release group root package.json. The CI pipeline will supply the build number and branch to determine the prerelease suffix if it is not a tagged build. ``` USAGE @@ -86,8 +86,7 @@ USAGE FLAGS -i, --includeInternalVersions= Include Fluid internal versions. - --base= The base version. This will be read from lerna.json/package.json if not - provided. + --base= The base version. This will be read from package.json if not provided. --build= (required) The CI build number. --packageTypes=