From d9290c4095405dc89a90c6aba6984b1de95f357f Mon Sep 17 00:00:00 2001 From: Abram Sanderson Date: Tue, 1 Aug 2023 09:45:01 -0700 Subject: [PATCH 1/8] Restore import/no-unresolved for ESM-only test packages (#16387) ## Description #15939 and related PRs disabled this linter rule for imports to `.js` files, since the base eslint module resolver doesn't understand that relative imports to .js should sometimes conceptually refer to .ts files instead. This re-enables linting for that rule in affected packages by using [eslint-import-resolver-typescript](https://www.npmjs.com/package/eslint-import-resolver-typescript). --- .../data-objects/table-document/.eslintrc.js | 17 +- .../data-objects/table-document/package.json | 2 + examples/data-objects/webflow/.eslintrc.cjs | 15 +- examples/data-objects/webflow/package.json | 2 + .../test/test-end-to-end-tests/.eslintrc.cjs | 15 +- .../test/test-end-to-end-tests/package.json | 2 + .../test/test-version-utils/.eslintrc.cjs | 15 +- packages/test/test-version-utils/package.json | 2 + pnpm-lock.yaml | 401 +++++++++++++++++- 9 files changed, 433 insertions(+), 38 deletions(-) diff --git a/examples/data-objects/table-document/.eslintrc.js b/examples/data-objects/table-document/.eslintrc.js index 31eca03a0423..0f72a83a3f5e 100644 --- a/examples/data-objects/table-document/.eslintrc.js +++ b/examples/data-objects/table-document/.eslintrc.js @@ -11,16 +11,13 @@ module.exports = { rules: { "@typescript-eslint/strict-boolean-expressions": "off", }, - overrides: [ - { - // Rules only for test files - files: ["*.spec.ts", "src/test/**"], - rules: { - // ESLint's resolver doesn't resolve relative imports of ESNext modules correctly, since - // it resolves the path relative to the .ts file (and assumes a file with a .js extension - // should exist there) - "import/no-unresolved": ["error", { ignore: ["^\\.(.*)\\.js$"] }], + settings: { + "import/resolver": { + // Use eslint-import-resolver-typescript. + // This ensures ESNext with `.js` extensions resolve correctly to their corresponding `.ts` files. + typescript: { + extensions: [".ts", ".tsx", ".d.ts", ".js", ".jsx"], }, }, - ], + }, }; diff --git a/examples/data-objects/table-document/package.json b/examples/data-objects/table-document/package.json index 16995cf6dc7f..9060a407a632 100644 --- a/examples/data-objects/table-document/package.json +++ b/examples/data-objects/table-document/package.json @@ -81,6 +81,8 @@ "concurrently": "^7.6.0", "cross-env": "^7.0.3", "eslint": "~8.6.0", + "eslint-import-resolver-typescript": "^3.5.5", + "eslint-plugin-import": "~2.25.4", "mocha": "^10.2.0", "mocha-json-output-reporter": "^2.0.1", "mocha-multi-reporters": "^1.5.1", diff --git a/examples/data-objects/webflow/.eslintrc.cjs b/examples/data-objects/webflow/.eslintrc.cjs index 7ab6fdb650b1..67a5d6346351 100644 --- a/examples/data-objects/webflow/.eslintrc.cjs +++ b/examples/data-objects/webflow/.eslintrc.cjs @@ -12,12 +12,15 @@ module.exports = { "max-len": "off", "no-bitwise": "off", "no-case-declarations": "off", - // ESLint's resolver doesn't resolve relative imports of ESNext modules correctly, since - // it resolves the path relative to the .ts file (and assumes a file with a .js extension - // should exist there) - // AB#4614 tracks moving to eslint-import-resolver-typescript (which handles such imports - // out of the box) and removing this exception. - "import/no-unresolved": ["error", { ignore: ["^\\.(.*)\\.js$"] }], + }, + settings: { + "import/resolver": { + // Use eslint-import-resolver-typescript. + // This ensures ESNext with `.js` extensions resolve correctly to their corresponding `.ts` files. + typescript: { + extensions: [".ts", ".tsx", ".d.ts", ".js", ".jsx"], + }, + }, }, parserOptions: { project: ["./tsconfig.json", "./src/test/tsconfig.json"], diff --git a/examples/data-objects/webflow/package.json b/examples/data-objects/webflow/package.json index 9996ceb8cffd..76fabf22421a 100644 --- a/examples/data-objects/webflow/package.json +++ b/examples/data-objects/webflow/package.json @@ -110,6 +110,8 @@ "cross-env": "^7.0.3", "css-loader": "^1.0.0", "eslint": "~8.6.0", + "eslint-import-resolver-typescript": "^3.5.5", + "eslint-plugin-import": "~2.25.4", "esm-loader-css": "^1.0.4", "file-loader": "^3.0.1", "html-loader": "^3.1.0", diff --git a/packages/test/test-end-to-end-tests/.eslintrc.cjs b/packages/test/test-end-to-end-tests/.eslintrc.cjs index e021e530a82f..5fc2ce6e09a3 100644 --- a/packages/test/test-end-to-end-tests/.eslintrc.cjs +++ b/packages/test/test-end-to-end-tests/.eslintrc.cjs @@ -15,6 +15,15 @@ module.exports = { // This library is used in the browser, so we don't want dependencies on most node libraries. "import/no-nodejs-modules": ["error", { allow: ["url"] }], }, + settings: { + "import/resolver": { + // Use eslint-import-resolver-typescript. + // This ensures ESNext with `.js` extensions resolve correctly to their corresponding `.ts` files. + typescript: { + extensions: [".ts", ".tsx", ".d.ts", ".js", ".jsx"], + }, + }, + }, overrides: [ { // Rules only for test files @@ -22,12 +31,6 @@ module.exports = { rules: { // Test files are run in node only so additional node libraries can be used. "import/no-nodejs-modules": ["error", { allow: ["assert", "url"] }], - // ESLint's resolver doesn't resolve relative imports of ESNext modules correctly, since - // it resolves the path relative to the .ts file (and assumes a file with a .js extension - // should exist there) - // AB#4614 tracks moving to eslint-import-resolver-typescript (which handles such imports - // out of the box) and removing this exception. - "import/no-unresolved": ["error", { ignore: ["^\\.(.*)\\.js$"] }], }, }, ], diff --git a/packages/test/test-end-to-end-tests/package.json b/packages/test/test-end-to-end-tests/package.json index 5a5026d255c0..1d3cb8181172 100644 --- a/packages/test/test-end-to-end-tests/package.json +++ b/packages/test/test-end-to-end-tests/package.json @@ -136,6 +136,8 @@ "@types/uuid": "^8.3.0", "concurrently": "^7.6.0", "eslint": "~8.6.0", + "eslint-import-resolver-typescript": "^3.5.5", + "eslint-plugin-import": "~2.25.4", "mocha-json-output-reporter": "^2.0.1", "mocha-multi-reporters": "^1.5.1", "moment": "^2.21.0", diff --git a/packages/test/test-version-utils/.eslintrc.cjs b/packages/test/test-version-utils/.eslintrc.cjs index 91279a164425..941104a5f000 100644 --- a/packages/test/test-version-utils/.eslintrc.cjs +++ b/packages/test/test-version-utils/.eslintrc.cjs @@ -8,12 +8,15 @@ module.exports = { rules: { "@typescript-eslint/strict-boolean-expressions": "off", // requires strictNullChecks=true in tsconfig "import/no-nodejs-modules": "off", - // ESLint's resolver doesn't resolve relative imports of ESNext modules correctly, since - // it resolves the path relative to the .ts file (and assumes a file with a .js extension - // should exist there) - // AB#4614 tracks moving to eslint-import-resolver-typescript (which handles such imports - // out of the box) and removing this exception. - "import/no-unresolved": ["error", { ignore: ["^\\.(.*)\\.(m|c)?js$"] }], + }, + settings: { + "import/resolver": { + // Use eslint-import-resolver-typescript. + // This ensures ESNext with `.js` extensions resolve correctly to their corresponding `.ts` files. + typescript: { + extensions: [".ts", ".tsx", ".d.ts", ".js", ".jsx"], + }, + }, }, parserOptions: { project: ["./tsconfig.json", "./src/test/tsconfig.json"], diff --git a/packages/test/test-version-utils/package.json b/packages/test/test-version-utils/package.json index e07b04682f65..285a6cfcea08 100644 --- a/packages/test/test-version-utils/package.json +++ b/packages/test/test-version-utils/package.json @@ -101,6 +101,8 @@ "concurrently": "^7.6.0", "cross-env": "^7.0.3", "eslint": "~8.6.0", + "eslint-import-resolver-typescript": "^3.5.5", + "eslint-plugin-import": "~2.25.4", "mocha": "^10.2.0", "mocha-json-output-reporter": "^2.0.1", "mocha-multi-reporters": "^1.5.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 838fb05a2741..e44ca6857e74 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2968,6 +2968,8 @@ importers: cross-env: ^7.0.3 debug: ^4.1.1 eslint: ~8.6.0 + eslint-import-resolver-typescript: ^3.5.5 + eslint-plugin-import: ~2.25.4 mocha: ^10.2.0 mocha-json-output-reporter: ^2.0.1 mocha-multi-reporters: ^1.5.1 @@ -2994,7 +2996,7 @@ importers: '@fluid-tools/build-cli': 0.21.0_@types+node@16.18.38 '@fluidframework/build-common': 1.2.0 '@fluidframework/build-tools': 0.21.0_@types+node@16.18.38 - '@fluidframework/eslint-config-fluid': 2.0.0_kufnqfq7tb5rpdawkdb6g5smma + '@fluidframework/eslint-config-fluid': 2.0.0_3t6p3jtubdnp6bfaldycrpk3ru '@fluidframework/mocha-test-setup': link:../../../packages/test/mocha-test-setup '@fluidframework/runtime-utils': link:../../../packages/runtime/runtime-utils '@fluidframework/test-runtime-utils': link:../../../packages/runtime/test-runtime-utils @@ -3005,6 +3007,8 @@ importers: concurrently: 7.6.0 cross-env: 7.0.3 eslint: 8.6.0 + eslint-import-resolver-typescript: 3.5.5_wwrwxdavgu4hzdoqb37x5rvvke + eslint-plugin-import: 2.25.4_av4jjatakjabmml5z5flrwsy3e mocha: 10.2.0 mocha-json-output-reporter: 2.1.0_mocha@10.2.0+moment@2.29.4 mocha-multi-reporters: 1.5.1_mocha@10.2.0 @@ -3207,6 +3211,8 @@ importers: css-loader: ^1.0.0 debug: ^4.1.1 eslint: ~8.6.0 + eslint-import-resolver-typescript: ^3.5.5 + eslint-plugin-import: ~2.25.4 esm-loader-css: ^1.0.4 events: ^3.1.0 file-loader: ^3.0.1 @@ -3256,7 +3262,7 @@ importers: '@fluid-tools/webpack-fluid-loader': link:../../../packages/tools/webpack-fluid-loader '@fluidframework/build-common': 1.2.0 '@fluidframework/build-tools': 0.21.0_2wska3rv6ra5ajbmktsvyxwga4 - '@fluidframework/eslint-config-fluid': 2.0.0_kufnqfq7tb5rpdawkdb6g5smma + '@fluidframework/eslint-config-fluid': 2.0.0_3t6p3jtubdnp6bfaldycrpk3ru '@fluidframework/mocha-test-setup': link:../../../packages/test/mocha-test-setup '@fluidframework/runtime-utils': link:../../../packages/runtime/runtime-utils '@fluidframework/test-utils': link:../../../packages/test/test-utils @@ -3270,6 +3276,8 @@ importers: cross-env: 7.0.3 css-loader: 1.0.1_webpack@5.83.0 eslint: 8.6.0 + eslint-import-resolver-typescript: 3.5.5_wwrwxdavgu4hzdoqb37x5rvvke + eslint-plugin-import: 2.25.4_av4jjatakjabmml5z5flrwsy3e esm-loader-css: 1.0.4 file-loader: 3.0.1_webpack@5.83.0 html-loader: 3.1.2_webpack@5.83.0 @@ -10331,6 +10339,8 @@ importers: concurrently: ^7.6.0 cross-env: ^7.0.3 eslint: ~8.6.0 + eslint-import-resolver-typescript: ^3.5.5 + eslint-plugin-import: ~2.25.4 mocha: ^10.2.0 mocha-json-output-reporter: ^2.0.1 mocha-multi-reporters: ^1.5.1 @@ -10403,13 +10413,15 @@ importers: '@fluid-tools/build-cli': 0.21.0_@types+node@16.18.38 '@fluidframework/build-common': 1.2.0 '@fluidframework/build-tools': 0.21.0_@types+node@16.18.38 - '@fluidframework/eslint-config-fluid': 2.0.0_kufnqfq7tb5rpdawkdb6g5smma + '@fluidframework/eslint-config-fluid': 2.0.0_3t6p3jtubdnp6bfaldycrpk3ru '@types/mocha': 9.1.1 '@types/nock': 9.3.1 '@types/node': 16.18.38 '@types/uuid': 8.3.4 concurrently: 7.6.0 eslint: 8.6.0 + eslint-import-resolver-typescript: 3.5.5_wwrwxdavgu4hzdoqb37x5rvvke + eslint-plugin-import: 2.25.4_av4jjatakjabmml5z5flrwsy3e mocha-json-output-reporter: 2.1.0_mocha@10.2.0+moment@2.29.4 mocha-multi-reporters: 1.5.1_mocha@10.2.0 moment: 2.29.4 @@ -10699,6 +10711,8 @@ importers: concurrently: ^7.6.0 cross-env: ^7.0.3 eslint: ~8.6.0 + eslint-import-resolver-typescript: ^3.5.5 + eslint-plugin-import: ~2.25.4 mocha: ^10.2.0 mocha-json-output-reporter: ^2.0.1 mocha-multi-reporters: ^1.5.1 @@ -10751,7 +10765,7 @@ importers: '@fluid-tools/build-cli': 0.21.0_2wska3rv6ra5ajbmktsvyxwga4 '@fluidframework/build-common': 1.2.0 '@fluidframework/build-tools': 0.21.0_2wska3rv6ra5ajbmktsvyxwga4 - '@fluidframework/eslint-config-fluid': 2.0.0_kufnqfq7tb5rpdawkdb6g5smma + '@fluidframework/eslint-config-fluid': 2.0.0_3t6p3jtubdnp6bfaldycrpk3ru '@fluidframework/mocha-test-setup': link:../mocha-test-setup '@types/mocha': 9.1.1 '@types/nock': 9.3.1 @@ -10761,6 +10775,8 @@ importers: concurrently: 7.6.0 cross-env: 7.0.3 eslint: 8.6.0 + eslint-import-resolver-typescript: 3.5.5_wwrwxdavgu4hzdoqb37x5rvvke + eslint-plugin-import: 2.25.4_av4jjatakjabmml5z5flrwsy3e mocha: 10.2.0 mocha-json-output-reporter: 2.1.0_mocha@10.2.0+moment@2.29.4 mocha-multi-reporters: 1.5.1_mocha@10.2.0 @@ -16248,6 +16264,32 @@ packages: - supports-color dev: true + /@fluidframework/eslint-config-fluid/2.0.0_3t6p3jtubdnp6bfaldycrpk3ru: + resolution: {integrity: sha512-/az5CybW5XUZUOk9HMH0nUMtKx5AK+CRfHg35UyygTK+V2OmNRes/yCAbmxoQ1J1Vn2iow3Y/Sgw/oJygciugQ==} + dependencies: + '@rushstack/eslint-patch': 1.1.4 + '@rushstack/eslint-plugin': 0.8.6_kufnqfq7tb5rpdawkdb6g5smma + '@rushstack/eslint-plugin-security': 0.2.6_kufnqfq7tb5rpdawkdb6g5smma + '@typescript-eslint/eslint-plugin': 5.9.1_i37r4pxnuonvhfobrnldva5ppi + '@typescript-eslint/parser': 5.9.1_kufnqfq7tb5rpdawkdb6g5smma + eslint-config-prettier: 8.5.0_eslint@8.6.0 + eslint-plugin-editorconfig: 3.2.0_4x3vxi7gdq53yv6dpzqqqrxppq + eslint-plugin-eslint-comments: 3.2.0_eslint@8.6.0 + eslint-plugin-import: 2.25.4_nfrjsm3ufztdjntgbuqnvazmai + eslint-plugin-jsdoc: 39.3.25_eslint@8.6.0 + eslint-plugin-promise: 6.0.1_eslint@8.6.0 + eslint-plugin-react: 7.28.0_eslint@8.6.0 + eslint-plugin-tsdoc: 0.2.17 + eslint-plugin-unicorn: 40.0.0_eslint@8.6.0 + eslint-plugin-unused-imports: 2.0.0_fhnxgfsp6r3qynjxjvskmntitm + transitivePeerDependencies: + - eslint + - eslint-import-resolver-typescript + - eslint-import-resolver-webpack + - supports-color + - typescript + dev: true + /@fluidframework/eslint-config-fluid/2.0.0_kufnqfq7tb5rpdawkdb6g5smma: resolution: {integrity: sha512-/az5CybW5XUZUOk9HMH0nUMtKx5AK+CRfHg35UyygTK+V2OmNRes/yCAbmxoQ1J1Vn2iow3Y/Sgw/oJygciugQ==} dependencies: @@ -19753,6 +19795,18 @@ packages: dev: true optional: true + /@pkgr/utils/2.4.2: + resolution: {integrity: sha512-POgTXhjrTfbTV63DiFXav4lBHiICLKKwDeaKn9Nphwj7WH6m0hMMCaJkMyRWjgtPFyRKRVoMXXjczsTQRDEhYw==} + engines: {node: ^12.20.0 || ^14.18.0 || >=16.0.0} + dependencies: + cross-spawn: 7.0.3 + fast-glob: 3.3.0 + is-glob: 4.0.3 + open: 9.1.0 + picocolors: 1.0.0 + tslib: 2.6.0 + dev: true + /@pmmmwh/react-refresh-webpack-plugin/0.5.10_bw64nqwvg33hl7aokjyon4wd5e: resolution: {integrity: sha512-j0Ya0hCFZPd4x40qLzbhGsh9TMtdb+CJQiso+WxLOPNasohq9cc5SNUcwsZaRH6++Xh91Xkm/xHCkuIiIu0LUA==} engines: {node: '>= 10.13'} @@ -21565,7 +21619,7 @@ packages: /@ts-morph/common/0.18.1: resolution: {integrity: sha512-RVE+zSRICWRsfrkAw5qCAK+4ZH9kwEFv5h0+/YeHTLieWP7F4wWq4JsKFuNWG+fYh/KF+8rAtgdj5zb2mm+DVA==} dependencies: - fast-glob: 3.2.12 + fast-glob: 3.3.0 minimatch: 5.1.6 mkdirp: 1.0.4 path-browserify: 1.0.1 @@ -24562,7 +24616,6 @@ packages: resolution: {integrity: sha512-GPEid2Y9QU1Exl1rpO9B2IPJGHPSupF5GnVIP0blYvNOMer2bTvSWs1jGOUg04hTmu67nmLsQ9TBo1puaotBHg==} engines: {node: '>=0.6'} dev: true - optional: true /big.js/5.2.2: resolution: {integrity: sha512-vyL2OymJxmarO8gxMr0mhChsO9QGwhynfuu4+MHTAW6czfq9humCB7rKpUjDd9YUiDPU4mzpyupFSvOClAwbmQ==} @@ -24725,6 +24778,13 @@ packages: dev: true optional: true + /bplist-parser/0.2.0: + resolution: {integrity: sha512-z0M+byMThzQmD9NILRniCUXYsYpjwnlO8N5uCFaCqIOpqRsJCrQL9NK3JsD67CN5a08nF5oIL2bD6loTdHOuKw==} + engines: {node: '>= 5.10.0'} + dependencies: + big-integer: 1.6.51 + dev: true + /brace-expansion/1.1.11: resolution: {integrity: sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==} dependencies: @@ -24945,6 +25005,13 @@ packages: dependencies: semver: 7.5.4 + /bundle-name/3.0.0: + resolution: {integrity: sha512-PKA4BeSvBpQKQ8iPOGCSiell+N8P+Tf1DlwqmYhpe2gAhKPHn8EYOxVT+ShuGmhg8lN8XiSlS80yiExKXrURlw==} + engines: {node: '>=12'} + dependencies: + run-applescript: 5.0.0 + dev: true + /bytes/3.0.0: resolution: {integrity: sha512-pMhOfFDPiv9t5jjIXkHosWmkSyQbvsgEVNkz0ERHbuLh2T/7j4Mqqpz523Fe8MVY89KC6Sh/QfS2sM+SjgFDcw==} engines: {node: '>= 0.8'} @@ -27185,6 +27252,24 @@ packages: dev: true optional: true + /default-browser-id/3.0.0: + resolution: {integrity: sha512-OZ1y3y0SqSICtE8DE4S8YOE9UZOJ8wO16fKWVP5J1Qz42kV9jcnMVFrEE/noXb/ss3Q4pZIH79kxofzyNNtUNA==} + engines: {node: '>=12'} + dependencies: + bplist-parser: 0.2.0 + untildify: 4.0.0 + dev: true + + /default-browser/4.0.0: + resolution: {integrity: sha512-wX5pXO1+BrhMkSbROFsyxUm0i/cJEScyNhA4PPxc41ICuv05ZZB/MX28s8aZx6xjmatvebIapF6hLEKEcpneUA==} + engines: {node: '>=14.16'} + dependencies: + bundle-name: 3.0.0 + default-browser-id: 3.0.0 + execa: 7.1.1 + titleize: 3.0.0 + dev: true + /default-gateway/6.0.3: resolution: {integrity: sha512-fwSOJsbbNzZ/CUFpqFBqYfYNLj1NbMPm8MMCIzHjC83iSJRBEGmDUxU+WP661BaBQImeC2yHwXtz+P/O9o+XEg==} engines: {node: '>= 10'} @@ -27216,6 +27301,11 @@ packages: resolution: {integrity: sha512-Ds09qNh8yw3khSjiJjiUInaGX9xlqZDY7JVryGxdxV7NPeuqQfplOpQ66yJFZut3jLa5zOwkXw1g9EI2uKh4Og==} engines: {node: '>=8'} + /define-lazy-prop/3.0.0: + resolution: {integrity: sha512-N+MeXYoqr3pOgn8xfyRPREN7gHakLYjhsHhWGT3fWAiL4IkAt0iDw14QiiEm2bE30c5XX5q0FtAA3CK5f9/BUg==} + engines: {node: '>=12'} + dev: true + /define-properties/1.2.0: resolution: {integrity: sha512-xvqAVKGfT1+UAvPwKTVw/njhdQ8ZhXK4lI0bCIuCMrp2up9nPnaDftrLtmpTazqd1o+UY4zgzU+avtMbDP+ldA==} engines: {node: '>= 0.4'} @@ -28140,6 +28230,30 @@ packages: - supports-color dev: true + /eslint-import-resolver-typescript/3.5.5_wwrwxdavgu4hzdoqb37x5rvvke: + resolution: {integrity: sha512-TdJqPHs2lW5J9Zpe17DZNQuDnox4xo2o+0tE7Pggain9Rbc19ik8kFtXdxZ250FVx2kF4vlt2RSf4qlUpG7bhw==} + engines: {node: ^14.18.0 || >=16.0.0} + peerDependencies: + eslint: '*' + eslint-plugin-import: '*' + dependencies: + debug: 4.3.4 + enhanced-resolve: 5.15.0 + eslint: 8.6.0 + eslint-module-utils: 2.8.0_av4jjatakjabmml5z5flrwsy3e + eslint-plugin-import: 2.25.4_av4jjatakjabmml5z5flrwsy3e + get-tsconfig: 4.6.2 + globby: 13.1.4 + is-core-module: 2.12.1 + is-glob: 4.0.3 + synckit: 0.8.5 + transitivePeerDependencies: + - '@typescript-eslint/parser' + - eslint-import-resolver-node + - eslint-import-resolver-webpack + - supports-color + dev: true + /eslint-module-utils/2.8.0_2wwsys6gbnsdc4a7jaa5rcv4nq: resolution: {integrity: sha512-aWajIYfsqCKRDgUfjEXNN/JlrzauMuSEy5sbd7WXbtW3EH6A6MpwEh42c7qD+MqQo9QMJ6fWLAeIJynx0g6OAw==} engines: {node: '>=4'} @@ -28168,6 +28282,63 @@ packages: - supports-color dev: true + /eslint-module-utils/2.8.0_ajv3vlqzjpoxbu6hdlpgvuycsm: + resolution: {integrity: sha512-aWajIYfsqCKRDgUfjEXNN/JlrzauMuSEy5sbd7WXbtW3EH6A6MpwEh42c7qD+MqQo9QMJ6fWLAeIJynx0g6OAw==} + engines: {node: '>=4'} + peerDependencies: + '@typescript-eslint/parser': '*' + eslint: '*' + eslint-import-resolver-node: '*' + eslint-import-resolver-typescript: '*' + eslint-import-resolver-webpack: '*' + peerDependenciesMeta: + '@typescript-eslint/parser': + optional: true + eslint: + optional: true + eslint-import-resolver-node: + optional: true + eslint-import-resolver-typescript: + optional: true + eslint-import-resolver-webpack: + optional: true + dependencies: + debug: 3.2.7 + eslint: 8.6.0 + eslint-import-resolver-node: 0.3.7 + eslint-import-resolver-typescript: 3.5.5_wwrwxdavgu4hzdoqb37x5rvvke + transitivePeerDependencies: + - supports-color + dev: true + + /eslint-module-utils/2.8.0_av4jjatakjabmml5z5flrwsy3e: + resolution: {integrity: sha512-aWajIYfsqCKRDgUfjEXNN/JlrzauMuSEy5sbd7WXbtW3EH6A6MpwEh42c7qD+MqQo9QMJ6fWLAeIJynx0g6OAw==} + engines: {node: '>=4'} + peerDependencies: + '@typescript-eslint/parser': '*' + eslint: '*' + eslint-import-resolver-node: '*' + eslint-import-resolver-typescript: '*' + eslint-import-resolver-webpack: '*' + peerDependenciesMeta: + '@typescript-eslint/parser': + optional: true + eslint: + optional: true + eslint-import-resolver-node: + optional: true + eslint-import-resolver-typescript: + optional: true + eslint-import-resolver-webpack: + optional: true + dependencies: + debug: 3.2.7 + eslint: 8.6.0 + eslint-import-resolver-typescript: 3.5.5_wwrwxdavgu4hzdoqb37x5rvvke + transitivePeerDependencies: + - supports-color + dev: true + /eslint-module-utils/2.8.0_iwcyz3q5swh44qq6nyzz4rzmcq: resolution: {integrity: sha512-aWajIYfsqCKRDgUfjEXNN/JlrzauMuSEy5sbd7WXbtW3EH6A6MpwEh42c7qD+MqQo9QMJ6fWLAeIJynx0g6OAw==} engines: {node: '>=4'} @@ -28197,6 +28368,36 @@ packages: - supports-color dev: true + /eslint-module-utils/2.8.0_wximnd5uvxmdvqrh5d74va2s3q: + resolution: {integrity: sha512-aWajIYfsqCKRDgUfjEXNN/JlrzauMuSEy5sbd7WXbtW3EH6A6MpwEh42c7qD+MqQo9QMJ6fWLAeIJynx0g6OAw==} + engines: {node: '>=4'} + peerDependencies: + '@typescript-eslint/parser': '*' + eslint: '*' + eslint-import-resolver-node: '*' + eslint-import-resolver-typescript: '*' + eslint-import-resolver-webpack: '*' + peerDependenciesMeta: + '@typescript-eslint/parser': + optional: true + eslint: + optional: true + eslint-import-resolver-node: + optional: true + eslint-import-resolver-typescript: + optional: true + eslint-import-resolver-webpack: + optional: true + dependencies: + '@typescript-eslint/parser': 5.9.1_kufnqfq7tb5rpdawkdb6g5smma + debug: 3.2.7 + eslint: 8.6.0 + eslint-import-resolver-node: 0.3.7 + eslint-import-resolver-typescript: 3.5.5_wwrwxdavgu4hzdoqb37x5rvvke + transitivePeerDependencies: + - supports-color + dev: true + /eslint-plugin-chai-expect/3.0.0_eslint@8.6.0: resolution: {integrity: sha512-NS0YBcToJl+BRKBSMCwRs/oHJIX67fG5Gvb4tGked+9Wnd1/PzKijd82B2QVKcSSOwRe+pp4RAJ2AULeck4eQw==} engines: {node: 10.* || 12.* || >= 14.*} @@ -28242,6 +28443,36 @@ packages: lodash.upperfirst: 4.3.1 dev: true + /eslint-plugin-import/2.25.4_av4jjatakjabmml5z5flrwsy3e: + resolution: {integrity: sha512-/KJBASVFxpu0xg1kIBn9AUa8hQVnszpwgE7Ld0lKAlx7Ie87yzEzCgSkekt+le/YVhiaosO4Y14GDAOc41nfxA==} + engines: {node: '>=4'} + peerDependencies: + '@typescript-eslint/parser': '*' + eslint: ^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8 + peerDependenciesMeta: + '@typescript-eslint/parser': + optional: true + dependencies: + array-includes: 3.1.6 + array.prototype.flat: 1.3.1 + debug: 2.6.9 + doctrine: 2.1.0 + eslint: 8.6.0 + eslint-import-resolver-node: 0.3.7 + eslint-module-utils: 2.8.0_ajv3vlqzjpoxbu6hdlpgvuycsm + has: 1.0.3 + is-core-module: 2.12.1 + is-glob: 4.0.3 + minimatch: 3.1.2 + object.values: 1.1.6 + resolve: 1.22.2 + tsconfig-paths: 3.14.2 + transitivePeerDependencies: + - eslint-import-resolver-typescript + - eslint-import-resolver-webpack + - supports-color + dev: true + /eslint-plugin-import/2.25.4_eslint@8.6.0: resolution: {integrity: sha512-/KJBASVFxpu0xg1kIBn9AUa8hQVnszpwgE7Ld0lKAlx7Ie87yzEzCgSkekt+le/YVhiaosO4Y14GDAOc41nfxA==} engines: {node: '>=4'} @@ -28303,6 +28534,37 @@ packages: - supports-color dev: true + /eslint-plugin-import/2.25.4_nfrjsm3ufztdjntgbuqnvazmai: + resolution: {integrity: sha512-/KJBASVFxpu0xg1kIBn9AUa8hQVnszpwgE7Ld0lKAlx7Ie87yzEzCgSkekt+le/YVhiaosO4Y14GDAOc41nfxA==} + engines: {node: '>=4'} + peerDependencies: + '@typescript-eslint/parser': '*' + eslint: ^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8 + peerDependenciesMeta: + '@typescript-eslint/parser': + optional: true + dependencies: + '@typescript-eslint/parser': 5.9.1_kufnqfq7tb5rpdawkdb6g5smma + array-includes: 3.1.6 + array.prototype.flat: 1.3.1 + debug: 2.6.9 + doctrine: 2.1.0 + eslint: 8.6.0 + eslint-import-resolver-node: 0.3.7 + eslint-module-utils: 2.8.0_wximnd5uvxmdvqrh5d74va2s3q + has: 1.0.3 + is-core-module: 2.12.1 + is-glob: 4.0.3 + minimatch: 3.1.2 + object.values: 1.1.6 + resolve: 1.22.2 + tsconfig-paths: 3.14.2 + transitivePeerDependencies: + - eslint-import-resolver-typescript + - eslint-import-resolver-webpack + - supports-color + dev: true + /eslint-plugin-jest/27.1.7_e7w73bdyuoy63spbnf7wdyrtai: resolution: {integrity: sha512-0QVzf+og4YI1Qr3UoprkqqhezAZjFffdi62b0IurkCXMqPtRW84/UT4CKsYT80h/D82LA9avjO/80Ou1LdgbaQ==} engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0} @@ -28838,6 +29100,21 @@ packages: signal-exit: 3.0.7 strip-final-newline: 2.0.0 + /execa/7.1.1: + resolution: {integrity: sha512-wH0eMf/UXckdUYnO21+HDztteVv05rq2GXksxT4fCGeHkBhw1DROXh40wcjMcRqDOWE7iPJ4n3M7e2+YFP+76Q==} + engines: {node: ^14.18.0 || ^16.14.0 || >=18.0.0} + dependencies: + cross-spawn: 7.0.3 + get-stream: 6.0.1 + human-signals: 4.3.1 + is-stream: 3.0.0 + merge-stream: 2.0.0 + npm-run-path: 5.1.0 + onetime: 6.0.0 + signal-exit: 3.0.7 + strip-final-newline: 3.0.0 + dev: true + /exit-on-epipe/1.0.1: resolution: {integrity: sha512-h2z5mrROTxce56S+pnvAV890uu7ls7f1kEvVGJbw1OlFH3/mlJ5bkXu0KRyW94v37zzHPiUd55iLn3DA7TjWpw==} engines: {node: '>=0.8'} @@ -29096,6 +29373,17 @@ packages: glob-parent: 5.1.2 merge2: 1.4.1 micromatch: 4.0.5 + dev: true + + /fast-glob/3.3.0: + resolution: {integrity: sha512-ChDuvbOypPuNjO8yIDf36x7BlZX1smcUMTTcyoIjycexOxd6DFsKsg21qVBzEmr3G7fUKIRy2/psii+CIUt7FA==} + engines: {node: '>=8.6.0'} + dependencies: + '@nodelib/fs.stat': 2.0.5 + '@nodelib/fs.walk': 1.2.8 + glob-parent: 5.1.2 + merge2: 1.4.1 + micromatch: 4.0.5 /fast-json-parse/1.0.3: resolution: {integrity: sha512-FRWsaZRWEJ1ESVNbDWmsAlqDk96gPQezzLghafp5J4GUKjbCz3OkAHuZs5TuPEtkbVQERysLp9xv6c24fBm8Aw==} @@ -29971,6 +30259,12 @@ packages: call-bind: 1.0.2 get-intrinsic: 1.2.1 + /get-tsconfig/4.6.2: + resolution: {integrity: sha512-E5XrT4CbbXcXWy+1jChlZmrmCwd5KGx502kDCXJJ7y898TtWW9FwoG5HfOLVRKmlmDGkWN2HM9Ho+/Y8F0sJDg==} + dependencies: + resolve-pkg-maps: 1.0.0 + dev: true + /get-value/2.0.6: resolution: {integrity: sha512-Ln0UQDlxH1BapMu3GPtf7CuYNwRZf2gwCuPqbyG6pB8WfmFpzqcy4xtAaAMUhnNqjMKTiCPZG2oMT3YSx8U2NA==} engines: {node: '>=0.10.0'} @@ -30200,7 +30494,7 @@ packages: '@types/glob': 7.2.0 array-union: 2.1.0 dir-glob: 3.0.1 - fast-glob: 3.2.12 + fast-glob: 3.3.0 glob: 7.2.3 ignore: 5.2.4 merge2: 1.4.1 @@ -30213,7 +30507,7 @@ packages: dependencies: array-union: 2.1.0 dir-glob: 3.0.1 - fast-glob: 3.2.12 + fast-glob: 3.3.0 ignore: 5.2.4 merge2: 1.4.1 slash: 3.0.0 @@ -30224,7 +30518,7 @@ packages: dependencies: array-union: 2.1.0 dir-glob: 3.0.1 - fast-glob: 3.2.12 + fast-glob: 3.3.0 ignore: 5.2.4 merge2: 1.4.1 slash: 3.0.0 @@ -30234,7 +30528,7 @@ packages: engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} dependencies: dir-glob: 3.0.1 - fast-glob: 3.2.12 + fast-glob: 3.3.0 ignore: 5.2.4 merge2: 1.4.1 slash: 4.0.0 @@ -31191,6 +31485,11 @@ packages: resolution: {integrity: sha512-B4FFZ6q/T2jhhksgkbEW3HBvWIfDW85snkQgawt07S7J5QXTk6BkNV+0yAeZrM5QpMAdYlocGoljn0sJ/WQkFw==} engines: {node: '>=10.17.0'} + /human-signals/4.3.1: + resolution: {integrity: sha512-nZXjEF2nbo7lIw3mgYjItAfgQXog3OjJogSbKa2CQIIvSGWcKgeJnQlNXip6NglNzYH45nSRiEVimMvYL8DDqQ==} + engines: {node: '>=14.18.0'} + dev: true + /humanize-ms/1.2.1: resolution: {integrity: sha512-Fl70vYtsAFb/C06PTS9dZBo7ihau+Tu/DNCk/OyHhea07S+aeMWpFFkUaXRa8fI+ScZbEI8dfSxwY7gxZ9SAVQ==} dependencies: @@ -31767,6 +32066,12 @@ packages: engines: {node: '>=8'} hasBin: true + /is-docker/3.0.0: + resolution: {integrity: sha512-eljcgEDlEns/7AXFosB5K/2nCM4P7FQPkGc/DWLy5rmFEWvZayGrik1d9/QIY5nJ4f9YsVvBkA6kJpHn9rISdQ==} + engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} + hasBin: true + dev: true + /is-dom/1.1.0: resolution: {integrity: sha512-u82f6mvhYxRPKpw8V1N0W8ce1xXwOrQtgGcxl6UCL5zBmZu3is/18K0rR7uFCnMDuAsS/3W54mGL4vsaFUQlEQ==} dependencies: @@ -31851,6 +32156,14 @@ packages: resolution: {integrity: sha512-FeXIBgG/CPGd/WUxuEyvgGTEfwiG9Z4EKGxjNMRqviiIIfsmgrpnHLffEDdwUHqNva1VEW91o3xBT/m8Elgl9g==} dev: false + /is-inside-container/1.0.0: + resolution: {integrity: sha512-KIYLCCJghfHZxqjYBE7rEy0OBuTd5xCHS7tHVgvCLkx7StIoaxwNW3hCALgEUjFfeRk+MG/Qxmp/vtETEF3tRA==} + engines: {node: '>=14.16'} + hasBin: true + dependencies: + is-docker: 3.0.0 + dev: true + /is-installed-globally/0.1.0: resolution: {integrity: sha512-ERNhMg+i/XgDwPIPF3u24qpajVreaiSuvpb1Uu0jugw7KKcxGyCX8cgp8P5fwTmAuXku6beDHHECdKArjlg7tw==} engines: {node: '>=4'} @@ -32053,6 +32366,11 @@ packages: resolution: {integrity: sha512-hFoiJiTl63nn+kstHGBtewWSKnQLpyb155KHheA1l39uvtO9nWIop1p3udqPcUd/xbF1VLMO4n7OI6p7RbngDg==} engines: {node: '>=8'} + /is-stream/3.0.0: + resolution: {integrity: sha512-LnQR4bZ9IADDRSkvpqMGvt/tEJWclzklNgSw48V5EAaAeDd6qGvN8ei6k5p0tvxSR171VmGyHuTiAOfxAbr8kA==} + engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} + dev: true + /is-string/1.0.7: resolution: {integrity: sha512-tE2UXzivje6ofPW7l23cjDOMa09gb7xlAqG6jG5ej6uPV32TlWP3NKPigtaGeHNu9fohccRYvIiZMfOOnOYUtg==} engines: {node: '>= 0.4'} @@ -34751,6 +35069,11 @@ packages: engines: {node: '>=8'} dev: true + /mimic-fn/4.0.0: + resolution: {integrity: sha512-vqiC06CuhBTUdZH+RYl8sFrL096vA45Ok5ISO6sE/Mr1jRbGH4Csnhi8f3wKVl7x8mO4Au7Ir9D3Oyv1VYMFJw==} + engines: {node: '>=12'} + dev: true + /mimic-response/1.0.1: resolution: {integrity: sha512-j5EctnkH7amfV/q5Hgmoal1g2QHFJRraOtmx0JpIqkxhBhI/lJSl1nMpQ45hVarwNETOoWEimndZ4QK0RHxuxQ==} engines: {node: '>=4'} @@ -35875,6 +36198,13 @@ packages: dependencies: path-key: 3.1.1 + /npm-run-path/5.1.0: + resolution: {integrity: sha512-sJOdmRGrY2sjNTRMbSvluQqg+8X7ZK61yvzBEIDhz4f8z1TZFYABsqjjCBd/0PUNE9M6QDgHJXQkGUEm7Q+l9Q==} + engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} + dependencies: + path-key: 4.0.0 + dev: true + /npm/9.8.0: resolution: {integrity: sha512-AXeiBAdfM5K2jvBwA7EGLKeYyt0VnhmJRnlq4k2+M0Ao9v7yKJBqF8xFPzQL8kAybzwlfpTPCZwM4uTIszb3xA==} engines: {node: ^14.17.0 || ^16.13.0 || >=18.0.0} @@ -36248,6 +36578,13 @@ packages: dependencies: mimic-fn: 2.1.0 + /onetime/6.0.0: + resolution: {integrity: sha512-1FlR+gjXK7X+AsAHso35MnyN5KqGwJRi/31ft6x0M194ht7S+rWAvd7PHss9xSKMzE0asv1pyIHaJYq+BbacAQ==} + engines: {node: '>=12'} + dependencies: + mimic-fn: 4.0.0 + dev: true + /open/6.4.0: resolution: {integrity: sha512-IFenVPgF70fSm1keSd2iDBIDIBZkroLeuffXq+wKTzTJlBpesFWojV9lb8mzOfaAzM1sr7HQHuO0vtV0zYekGg==} engines: {node: '>=8'} @@ -36271,6 +36608,16 @@ packages: is-docker: 2.2.1 is-wsl: 2.2.0 + /open/9.1.0: + resolution: {integrity: sha512-OS+QTnw1/4vrf+9hh1jc1jnYjzSG4ttTBB8UxOwAnInG3Uo4ssetzC1ihqaIHjLJnA5GGlRl6QlZXOTQhRBUvg==} + engines: {node: '>=14.16'} + dependencies: + default-browser: 4.0.0 + define-lazy-prop: 3.0.0 + is-inside-container: 1.0.0 + is-wsl: 2.2.0 + dev: true + /opener/1.5.2: resolution: {integrity: sha512-ur5UIdyw5Y7yEj9wLzhqXiy6GZ3Mwx0yGI+5sMn2r0N0v3cKJvUmFH5yPP+WXh9e0xfyzyJX95D8l088DNFj7A==} hasBin: true @@ -36863,6 +37210,11 @@ packages: resolution: {integrity: sha512-ojmeN0qd+y0jszEtoY48r0Peq5dwMEkIlCOu6Q5f41lfkswXuKtYrhgoTpLnyIcHm24Uhqx+5Tqm2InSwLhE6Q==} engines: {node: '>=8'} + /path-key/4.0.0: + resolution: {integrity: sha512-haREypq7xkM7ErfgIyA0z+Bj4AGKlMSdlQE2jvJo6huWD1EdkKYV+G/T4nq0YEF2vgTT8kqMFKo1uHn950r4SQ==} + engines: {node: '>=12'} + dev: true + /path-parse/1.0.7: resolution: {integrity: sha512-LDJzPVEEEPR+y48z93A0Ed0yXb8pAByGWo/k5YYdYgpY2/2EsOsksJrq7lOHxryrVOn1ejG6oAp8ahvOIQD8sw==} @@ -39000,6 +39352,10 @@ packages: resolution: {integrity: sha512-qYg9KP24dD5qka9J47d0aVky0N+b4fTU89LN9iDnjB5waksiC49rvMB0PrUJQGoTmH50XPiqOvAjDfaijGxYZw==} engines: {node: '>=8'} + /resolve-pkg-maps/1.0.0: + resolution: {integrity: sha512-seS2Tj26TBVOC2NIc2rOe2y2ZO7efxITtLZcGSOnHHNOQ7CkiUBfw0Iw2ck6xkIhPwLhKNLS8BO+hEpngQlqzw==} + dev: true + /resolve-url/0.2.1: resolution: {integrity: sha512-ZuF55hVUQaaczgOIwqWzkEcEidmlD/xl44x1UZnhOXcYuFN2S6+rcxpG+C1N3So0wvNI3DmJICUFfu2SxhBmvg==} deprecated: https://github.com/lydell/resolve-url#deprecated @@ -39157,6 +39513,13 @@ packages: '@babel/runtime': 7.21.5 dev: false + /run-applescript/5.0.0: + resolution: {integrity: sha512-XcT5rBksx1QdIhlFOCtgZkB99ZEouFZ1E2Kc2LHqNW13U3/74YGdkQRmThTwxy4QIyookibDKYZOPqX//6BlAg==} + engines: {node: '>=12'} + dependencies: + execa: 5.1.1 + dev: true + /run-async/2.4.1: resolution: {integrity: sha512-tvVnVv01b8c1RrA6Ep7JkStj85Guv/YrMcwqYQnwjsAS2cTmmPGBBjAjpCW7RrSodNSoE2/qg9O4bceNvUuDgQ==} engines: {node: '>=0.12.0'} @@ -40602,6 +40965,11 @@ packages: resolution: {integrity: sha512-BrpvfNAE3dcvq7ll3xVumzjKjZQ5tI1sEUIKr3Uoks0XUl45St3FlatVqef9prk4jRDzhW6WZg+3bk93y6pLjA==} engines: {node: '>=6'} + /strip-final-newline/3.0.0: + resolution: {integrity: sha512-dOESqjYr96iWYylGObzd39EuNTa5VJxyvVAEm5Jnh7KGo75V43Hk1odPQkNDyXNmUR6k+gEiDVXnjB8HJ3crXw==} + engines: {node: '>=12'} + dev: true + /strip-indent/1.0.1: resolution: {integrity: sha512-I5iQq6aFMM62fBEAIB/hXzwJD6EEZ0xEGCX2t7oXqaKPIRgt4WruAQ285BISgdkP+HLGWyeGmNJcpIwFeRYRUA==} engines: {node: '>=0.10.0'} @@ -40897,6 +41265,14 @@ packages: resolution: {integrity: sha512-AsS729u2RHUfEra9xJrE39peJcc2stq2+poBXX8bcM08Y6g9j/i/PUzwNQqkaJde7Ntg1TO7bSREbR5sdosQ+g==} dev: true + /synckit/0.8.5: + resolution: {integrity: sha512-L1dapNV6vu2s/4Sputv8xGsCdAVlb5nRDMFU/E27D44l5U6cw1g0dGd45uLc+OXjNMmF4ntiMdCimzcjFKQI8Q==} + engines: {node: ^14.18.0 || >=16.0.0} + dependencies: + '@pkgr/utils': 2.4.2 + tslib: 2.6.0 + dev: true + /syncpack/9.8.6: resolution: {integrity: sha512-4S4cUoKK9WenA/Wdk9GvlekzPR9PxC7sqcsUIsK4ypsa/pIYv8Ju1vxGNvp6Y1yI2S9EdCk0QJsB3/wRB8XYVw==} engines: {node: '>=14'} @@ -41309,6 +41685,11 @@ packages: dependencies: tslib: 2.6.0 + /titleize/3.0.0: + resolution: {integrity: sha512-KxVu8EYHDPBdUYdKZdKtU2aj2XfEx9AfjXxE/Aj0vT06w2icA09Vus1rh6eSu1y01akYg6BjIK/hxyLJINoMLQ==} + engines: {node: '>=12'} + dev: true + /tmp/0.0.33: resolution: {integrity: sha512-jRCJlojKnZ3addtTOjdIqoRuPEKBvNXcGYqzO6zWZX8KfKEpnGY5jfggJQ3EjKuu8D4bJRr0y+cYJFmYbImXGw==} engines: {node: '>=0.6.0'} From 3f07bce3f84dd7b0d21393dbad19c5a7a6b37d0a Mon Sep 17 00:00:00 2001 From: Noah Encke <78610362+noencke@users.noreply.github.com> Date: Tue, 1 Aug 2023 10:14:10 -0700 Subject: [PATCH 2/8] Fix incorrect handling of detached commits in EditManager (#16629) ## Description Before a SharedTree is attached, any local commits are "round tripped" to the EditManager locally by immediately marking them as sequenced. These commits use "fake" sequence numbers that are very large negative integers. However, the EditManager expects its sequenced commits to have positive integers for the sake of trunk eviction. This caused a problem; commits submitted before attach were getting tracked as having occurred _before_ the latest known minimum sequence number, and thus when trunk eviction occurred, it considered them to already have been evicted even though they weren't yet. Essentially, an implicit invariant had been broken in the EditManager: sequenced commits must have a sequence number that is greater than the last known minimum sequence number. This implicit invariant has been made explicit to prevent this error from happening again in the future, and the default minimum sequence number in EditManager has been aligned with the one in SharedTreeCore so that they work together. --- .../dds/tree2/src/shared-tree-core/editManager.ts | 13 ++++++++++--- .../src/test/shared-tree-core/editManager.spec.ts | 14 +++++++------- .../tree2/src/test/shared-tree/sharedTree.spec.ts | 3 +-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/experimental/dds/tree2/src/shared-tree-core/editManager.ts b/experimental/dds/tree2/src/shared-tree-core/editManager.ts index 1b64cd357827..5165385c6fd0 100644 --- a/experimental/dds/tree2/src/shared-tree-core/editManager.ts +++ b/experimental/dds/tree2/src/shared-tree-core/editManager.ts @@ -89,8 +89,11 @@ export class EditManager< Set> >(); - /** The sequence number of the newest commit on the trunk that has been received by all peers */ - private minimumSequenceNumber: SeqNumber = brand(-1); + /** + * The sequence number of the newest commit on the trunk that has been received by all peers. + * Defaults to {@link minimumPossibleSequenceNumber} if no commits have been received. + */ + private minimumSequenceNumber = minimumPossibleSequenceNumber; /** * An immutable "origin" commit singleton on which the trunk is based. @@ -300,7 +303,7 @@ export class EditManager< this.trunk.getHead() === this.trunkBase && this.peerLocalBranches.size === 0 && this.localBranch.getHead() === this.trunk.getHead() && - this.minimumSequenceNumber === -1 + this.minimumSequenceNumber === minimumPossibleSequenceNumber ); } @@ -440,6 +443,10 @@ export class EditManager< sequenceNumber: SeqNumber, referenceSequenceNumber: SeqNumber, ): void { + assert( + sequenceNumber > this.minimumSequenceNumber, + "Expected change sequence number to exceed the last known minimum sequence number", + ); if (newCommit.sessionId === this.localSessionId) { const [firstLocalCommit] = getPathFromBase( this.localBranch.getHead(), diff --git a/experimental/dds/tree2/src/test/shared-tree-core/editManager.spec.ts b/experimental/dds/tree2/src/test/shared-tree-core/editManager.spec.ts index 4e6cd1acdd0b..14e8421a0f48 100644 --- a/experimental/dds/tree2/src/test/shared-tree-core/editManager.spec.ts +++ b/experimental/dds/tree2/src/test/shared-tree-core/editManager.spec.ts @@ -269,24 +269,24 @@ describe("EditManager", () => { it("Evicts trunk commits according to a provided minimum sequence number", () => { const { manager } = editManagerFactory({}); - for (let i = 0; i < 10; ++i) { + for (let i = 1; i <= 10; ++i) { manager.addSequencedChange(applyLocalCommit(manager), brand(i), brand(i - 1)); } assert.equal(manager.getTrunkChanges().length, 10); manager.advanceMinimumSequenceNumber(brand(5)); - assert.equal(manager.getTrunkChanges().length, 5); + assert.equal(manager.getTrunkChanges().length, 6); manager.advanceMinimumSequenceNumber(brand(10)); - assert.equal(manager.getTrunkChanges().length, 0); - for (let i = 10; i < 20; ++i) { + assert.equal(manager.getTrunkChanges().length, 1); + for (let i = 11; i <= 20; ++i) { manager.addSequencedChange(applyLocalCommit(manager), brand(i), brand(i - 1)); } - assert.equal(manager.getTrunkChanges().length, 10); + assert.equal(manager.getTrunkChanges().length, 11); manager.advanceMinimumSequenceNumber(brand(15)); - assert.equal(manager.getTrunkChanges().length, 5); + assert.equal(manager.getTrunkChanges().length, 6); manager.advanceMinimumSequenceNumber(brand(20)); - assert.equal(manager.getTrunkChanges().length, 0); + assert.equal(manager.getTrunkChanges().length, 1); }); it("Evicts trunk commits after but not exactly at the minimum sequence number", () => { diff --git a/experimental/dds/tree2/src/test/shared-tree/sharedTree.spec.ts b/experimental/dds/tree2/src/test/shared-tree/sharedTree.spec.ts index a3b0bfb5ae47..d0f5b1330f2d 100644 --- a/experimental/dds/tree2/src/test/shared-tree/sharedTree.spec.ts +++ b/experimental/dds/tree2/src/test/shared-tree/sharedTree.spec.ts @@ -1806,8 +1806,7 @@ describe("SharedTree", () => { child.transaction.start(); pushTestValueDirect(child, "C"); child.transaction.commit(); - // TODO:#4925: It should not be necessary to keep the child undisposed here. - parent.merge(child, false); + parent.merge(child); assert.deepEqual(getTestValues(parent), ["A", "B", "C"]); }; const provider = await TestTreeProvider.create( From c1bb43e5a5f1b8cdc2844d9bd7533179459fd7a6 Mon Sep 17 00:00:00 2001 From: Kian Thompson <102998837+kian-thompson@users.noreply.github.com> Date: Tue, 1 Aug 2023 10:25:34 -0700 Subject: [PATCH 3/8] Stop listening to DeltaManager op event for summary heuristics (#16628) Running summarizer should listen to ContainerRuntime "op" event explicitly, but this requires all ops to be passed from loader to runtime (version "2.0.0-internal.1.2.0" or later https://github.com/microsoft/FluidFramework/pull/11832). 99.9% of sessions are now running on a loader version of at least "2.0.0-internal.1.2.0". [AB#3883](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/3883) --- .../container-runtime/src/containerRuntime.ts | 4 +--- .../src/summary/runningSummarizer.ts | 23 +++---------------- 2 files changed, 4 insertions(+), 23 deletions(-) diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 3cbff12ca828..978673100bf8 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -2186,9 +2186,7 @@ export class ContainerRuntime } } - if (runtimeMessage || this.groupedBatchingEnabled) { - this.emit("op", message, runtimeMessage); - } + this.emit("op", message, runtimeMessage); this.scheduleManager.afterOpProcessing(undefined, message); diff --git a/packages/runtime/container-runtime/src/summary/runningSummarizer.ts b/packages/runtime/container-runtime/src/summary/runningSummarizer.ts index 2cb7b965420a..edb6ecc7e3f6 100644 --- a/packages/runtime/container-runtime/src/summary/runningSummarizer.ts +++ b/packages/runtime/container-runtime/src/summary/runningSummarizer.ts @@ -13,7 +13,6 @@ import { import { assert, delay, Deferred, PromiseTimer } from "@fluidframework/common-utils"; import { UsageError } from "@fluidframework/container-utils"; import { DriverErrorType } from "@fluidframework/driver-definitions"; -import { isRuntimeMessage } from "@fluidframework/driver-utils"; import { ISequencedDocumentMessage, MessageType } from "@fluidframework/protocol-definitions"; import { ISummaryConfiguration } from "../containerRuntime"; import { opSize } from "../opProperties"; @@ -148,7 +147,6 @@ export class RunningSummarizer implements IDisposable { private totalSuccessfulAttempts = 0; private initialized = false; - private readonly deltaManagerListener; private readonly runtimeListener; private constructor( @@ -239,24 +237,10 @@ export class RunningSummarizer implements IDisposable { this.mc.logger, ); - // Listen to deltaManager for non-runtime ops - this.deltaManagerListener = (op) => { - if (!isRuntimeMessage(op)) { - this.handleOp(op, false); - } + // Listen to runtime for ops + this.runtimeListener = (op: ISequencedDocumentMessage, runtimeMessage?: boolean) => { + this.handleOp(op, runtimeMessage === true); }; - - // Listen to runtime for runtime ops - this.runtimeListener = (op, runtimeMessage) => { - if (runtimeMessage) { - this.handleOp(op, true); - } - }; - - // Purpose of listening to deltaManager is for back-compat - // Can remove and only listen to runtime once loader version is past 2.0.0-internal.1.2.0 (https://github.com/microsoft/FluidFramework/pull/11832) - // Tracked by AB#3883 - this.runtime.deltaManager.on("op", this.deltaManagerListener); this.runtime.on("op", this.runtimeListener); } @@ -351,7 +335,6 @@ export class RunningSummarizer implements IDisposable { } public dispose(): void { - this.runtime.deltaManager.off("op", this.deltaManagerListener); this.runtime.off("op", this.runtimeListener); this.summaryWatcher.dispose(); this.heuristicRunner?.dispose(); From 55543625917687fbc85848f3b6dfdd4cf13f13ea Mon Sep 17 00:00:00 2001 From: "Craig Macomber (Microsoft)" <42876482+CraigMacomber@users.noreply.github.com> Date: Tue, 1 Aug 2023 10:31:54 -0700 Subject: [PATCH 4/8] tree2: More accurate schema and less verbose data for forest test suite (#16631) ## Description The forest test suite would construct out of schema forests, for example putting two objects in a field of an object in the json domain, and having an empty root schema with non empty documents. This has been fixed (at least in several cases: the whole suite has not been fully audited and no dynamic enforcement is present). Additionally, the tests used the longer JsonableTree format instead of the Json domain specific Json format or contextually typed data. This has been changed to make the tests shorter. --- .../dds/tree2/src/test/forestTestSuite.ts | 254 ++++++------------ 1 file changed, 77 insertions(+), 177 deletions(-) diff --git a/experimental/dds/tree2/src/test/forestTestSuite.ts b/experimental/dds/tree2/src/test/forestTestSuite.ts index 2d93dccd4429..b7b9f2a4b1d0 100644 --- a/experimental/dds/tree2/src/test/forestTestSuite.ts +++ b/experimental/dds/tree2/src/test/forestTestSuite.ts @@ -21,18 +21,19 @@ import { UpPath, clonePath, ITreeCursor, + EmptyKey, + ValueSchema, } from "../core"; import { cursorToJsonObject, jsonNumber, - jsonObject, jsonSchema, jsonRoot, singleJsonCursor, jsonBoolean, jsonString, } from "../domains"; -import { brand, brandOpaque } from "../util"; +import { JsonCompatible, brand, brandOpaque } from "../util"; import { FieldKinds, jsonableTreeFromCursor, @@ -40,6 +41,7 @@ import { defaultSchemaPolicy, isNeverField, SchemaBuilder, + cursorForTypedTreeData, } from "../feature-libraries"; import { MockDependent } from "./utils"; import { testGeneralPurposeTreeCursor, testTreeSchema } from "./cursorTestSuite"; @@ -62,35 +64,20 @@ export interface ForestTestConfiguration { skipCursorErrorCheck?: true; } +const jsonDocumentSchema = new SchemaBuilder("jsonDocumentSchema", jsonSchema).intoDocumentSchema( + SchemaBuilder.fieldSequence(...jsonRoot), +); + /** * Generic forest test suite */ export function testForest(config: ForestTestConfiguration): void { const factory = config.factory; describe(config.suiteName, () => { - const nestedContent: JsonableTree[] = [ - { - type: jsonObject.name, - fields: { - x: [ - { - type: jsonNumber.name, - value: 0, - fields: { - foo: [{ type: jsonNumber.name, value: 1 }], - }, - }, - ], - y: [ - { - type: jsonNumber.name, - value: 1, - }, - ], - }, - }, - ]; - + const nestedContent: JsonCompatible = { + x: { foo: 2 }, + y: 1, + }; const xField = brand("x"); const yField = brand("y"); const fooField: FieldKey = brand("foo"); @@ -132,19 +119,10 @@ export function testForest(config: ForestTestConfiguration): void { }); it("cursor use", () => { - const content: JsonableTree = { - type: jsonObject.name, - fields: { - data: [ - { type: jsonNumber.name, value: 1 }, - { type: jsonNumber.name, value: 2 }, - ], - }, - }; const forest = factory( new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), ); - initializeForest(forest, [singleTextCursor(content)]); + initializeForest(forest, [singleJsonCursor([1, 2])]); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -156,11 +134,11 @@ export function testForest(config: ForestTestConfiguration): void { assert.deepEqual(reader.getPath()?.parent, reader2.getFieldPath().parent); assert(reader2.firstNode()); assert.deepEqual(reader.getPath(), reader2.getPath()); - reader.enterField(brand("data")); + reader.enterField(EmptyKey); reader.enterNode(1); assert.equal(reader.value, 2); // Move reader two down to the same place, but by a different route. - reader2.enterField(brand("data")); + reader2.enterField(EmptyKey); reader2.enterNode(0); assert.equal(reader2.value, 1); assert.equal(reader.value, 2); @@ -184,29 +162,18 @@ export function testForest(config: ForestTestConfiguration): void { it("anchors creation and use", () => { const forest = factory( - new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), + new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonDocumentSchema), ); const dependent = new MockDependent("dependent"); recordDependency(dependent, forest); - const content: JsonableTree[] = [ - { - type: jsonObject.name, - fields: { - data: [ - { type: jsonNumber.name, value: 1 }, - { type: jsonNumber.name, value: 2 }, - ], - }, - }, - ]; - initializeForest(forest, content.map(singleTextCursor)); + initializeForest(forest, [singleJsonCursor([1, 2])]); const cursor = forest.allocateCursor(); moveToDetachedField(forest, cursor); assert(cursor.firstNode()); const parentAnchor = cursor.buildAnchor(); - cursor.enterField(brand("data")); + cursor.enterField(EmptyKey); cursor.enterNode(0); assert.equal(cursor.value, 1); const childAnchor1 = cursor.buildAnchor(); @@ -232,13 +199,13 @@ export function testForest(config: ForestTestConfiguration): void { const expectedChild1: UpPath = { parent: expectedParent, - parentField: brand("data"), + parentField: EmptyKey, parentIndex: 0, }; const expectedChild2: UpPath = { parent: expectedParent, - parentField: brand("data"), + parentField: EmptyKey, parentIndex: 1, }; @@ -263,28 +230,17 @@ export function testForest(config: ForestTestConfiguration): void { it("using an anchor that went away returns NotFound", () => { const forest = factory( - new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), + new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonDocumentSchema), ); const dependent = new MockDependent("dependent"); recordDependency(dependent, forest); - const content: JsonableTree[] = [ - { - type: jsonObject.name, - fields: { - data: [ - { type: jsonNumber.name, value: 1 }, - { type: jsonNumber.name, value: 2 }, - ], - }, - }, - ]; - initializeForest(forest, content.map(singleTextCursor)); + initializeForest(forest, [singleJsonCursor([1, 2])]); const cursor = forest.allocateCursor(); moveToDetachedField(forest, cursor); assert(cursor.firstNode()); - cursor.enterField(brand("data")); + cursor.enterField(EmptyKey); cursor.enterNode(0); const firstNodeAnchor = cursor.buildAnchor(); cursor.clear(); @@ -312,14 +268,10 @@ export function testForest(config: ForestTestConfiguration): void { it("primitive nodes", () => { const forest = factory( - new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), + new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonDocumentSchema), ); - const content: JsonableTree[] = [ - { type: jsonNumber.name, value: 1 }, - { type: jsonBoolean.name, value: true }, - { type: jsonString.name, value: "test" }, - ]; - initializeForest(forest, content.map(singleTextCursor)); + const content: JsonCompatible[] = [1, true, "test"]; + initializeForest(forest, content.map(singleJsonCursor)); const clone = forest.clone(forest.schema, forest.anchors); const reader = clone.allocateCursor(); @@ -335,30 +287,23 @@ export function testForest(config: ForestTestConfiguration): void { it("multiple fields", () => { const forest = factory( - new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), + new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonDocumentSchema), ); - initializeForest(forest, nestedContent.map(singleTextCursor)); + initializeForest(forest, [singleJsonCursor(nestedContent)]); const clone = forest.clone(forest.schema, forest.anchors); const reader = clone.allocateCursor(); moveToDetachedField(clone, reader); assert(reader.firstNode()); - reader.enterField(xField); - assert(reader.firstNode()); - assert.equal(reader.value, 0); - assert.equal(reader.nextNode(), false); - reader.exitField(); - reader.enterField(yField); - assert(reader.firstNode()); - assert.equal(reader.value, 1); - assert.equal(reader.nextNode(), false); + const fromClone = cursorToJsonObject(reader); + assert.deepEqual(nestedContent, fromClone); }); it("with anchors", () => { const forest = factory( new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), ); - initializeForest(forest, nestedContent.map(singleTextCursor)); + initializeForest(forest, [singleJsonCursor(nestedContent)]); const forestReader = forest.allocateCursor(); moveToDetachedField(forest, forestReader); @@ -370,7 +315,7 @@ export function testForest(config: ForestTestConfiguration): void { const clone = forest.clone(forest.schema, forest.anchors); const reader = clone.allocateCursor(); clone.tryMoveCursorToNode(anchor, reader); - assert.equal(reader.value, 0); + assert.equal(reader.value, undefined); }); }); @@ -409,8 +354,7 @@ export function testForest(config: ForestTestConfiguration): void { const forest = factory( new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), ); - const content: JsonableTree[] = [{ type: jsonNumber.name, value: 1 }]; - initializeForest(forest, content.map(singleTextCursor)); + initializeForest(forest, [singleJsonCursor(1)]); const cursor = forest.allocateCursor(); moveToDetachedField(forest, cursor); @@ -424,7 +368,7 @@ export function testForest(config: ForestTestConfiguration): void { const forest = factory( new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), ); - initializeForest(forest, nestedContent.map(singleTextCursor)); + initializeForest(forest, [singleJsonCursor(nestedContent)]); const setField: Delta.Modify = { type: Delta.MarkType.Modify, @@ -459,13 +403,10 @@ export function testForest(config: ForestTestConfiguration): void { it("delete", () => { const forest = factory( - new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), + new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonDocumentSchema), ); - const content: JsonableTree[] = [ - { type: jsonNumber.name, value: 1 }, - { type: jsonNumber.name, value: 2 }, - ]; - initializeForest(forest, content.map(singleTextCursor)); + const content: JsonCompatible[] = [1, 2]; + initializeForest(forest, content.map(singleJsonCursor)); const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [0, mark]]]); @@ -481,13 +422,10 @@ export function testForest(config: ForestTestConfiguration): void { it("a skip", () => { const forest = factory( - new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), + new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonDocumentSchema), ); - const content: JsonableTree[] = [ - { type: jsonNumber.name, value: 1 }, - { type: jsonNumber.name, value: 2 }, - ]; - initializeForest(forest, content.map(singleTextCursor)); + const content: JsonCompatible[] = [1, 2]; + initializeForest(forest, content.map(singleJsonCursor)); const cursor = forest.allocateCursor(); moveToDetachedField(forest, cursor); cursor.firstNode(); @@ -508,17 +446,14 @@ export function testForest(config: ForestTestConfiguration): void { it("insert", () => { const forest = factory( - new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), + new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonDocumentSchema), ); - const content: JsonableTree[] = [ - { type: jsonNumber.name, value: 1 }, - { type: jsonNumber.name, value: 2 }, - ]; - initializeForest(forest, content.map(singleTextCursor)); + const content: JsonCompatible[] = [1, 2]; + initializeForest(forest, content.map(singleJsonCursor)); const mark: Delta.Insert = { type: Delta.MarkType.Insert, - content: [singleTextCursor({ type: jsonNumber.name, value: 3 })], + content: [singleJsonCursor(3)], }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); forest.applyDelta(delta); @@ -536,7 +471,7 @@ export function testForest(config: ForestTestConfiguration): void { it("move-out under transient node", () => { const forest = factory( - new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), + new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonDocumentSchema), ); const moveId: Delta.MoveId = brand(1); @@ -553,19 +488,7 @@ export function testForest(config: ForestTestConfiguration): void { }; const mark: Delta.Insert = { type: Delta.MarkType.Insert, - content: [ - singleTextCursor({ - type: jsonObject.name, - fields: { - x: [ - { - type: jsonNumber.name, - value: 0, - }, - ], - }, - }), - ], + content: [singleJsonCursor({ x: 0 })], isTransient: true, fields: new Map([[xField, [moveOut]]]), }; @@ -583,7 +506,7 @@ export function testForest(config: ForestTestConfiguration): void { const forest = factory( new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), ); - initializeForest(forest, nestedContent.map(singleTextCursor)); + initializeForest(forest, [singleJsonCursor(nestedContent)]); const moveId = brandOpaque(0); const moveOut: Delta.MoveOut = { @@ -619,11 +542,8 @@ export function testForest(config: ForestTestConfiguration): void { const forest = factory( new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), ); - const content: JsonableTree[] = [ - { type: jsonNumber.name, value: 1 }, - { type: jsonNumber.name, value: 2 }, - ]; - initializeForest(forest, content.map(singleTextCursor)); + const content: JsonCompatible[] = [1, 2]; + initializeForest(forest, content.map(singleJsonCursor)); const mark: Delta.Insert = { type: Delta.MarkType.Insert, @@ -665,7 +585,7 @@ export function testForest(config: ForestTestConfiguration): void { const forest = factory( new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), ); - initializeForest(forest, nestedContent.map(singleTextCursor)); + initializeForest(forest, [singleJsonCursor(nestedContent)]); const moveId = brandOpaque(0); const mark: Delta.Delete = { @@ -683,7 +603,7 @@ export function testForest(config: ForestTestConfiguration): void { const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); assert.equal(reader.firstNode(), true); - assert.equal(reader.value, 0); + assert.equal(reader.value, undefined); assert.equal(reader.firstField(), true); assert.equal(reader.getFieldKey(), fooField); }); @@ -692,7 +612,7 @@ export function testForest(config: ForestTestConfiguration): void { const forest = factory( new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), ); - initializeForest(forest, nestedContent.map(singleTextCursor)); + initializeForest(forest, [singleJsonCursor(nestedContent)]); const moveId = brandOpaque(0); const mark: Delta.Modify = { @@ -712,12 +632,7 @@ export function testForest(config: ForestTestConfiguration): void { { type: Delta.MarkType.Delete, count: 1 }, { type: Delta.MarkType.Insert, - content: [ - singleTextCursor({ - type: jsonNumber.name, - value: 2, - }), - ], + content: [singleJsonCursor(2)], }, ], ], @@ -756,10 +671,9 @@ export function testForest(config: ForestTestConfiguration): void { const dependent = new MockDependent("dependent"); recordDependency(dependent, forest); - const content: JsonableTree[] = [{ type: jsonNumber.name, value: 1 }]; const insert: Delta.Insert = { type: Delta.MarkType.Insert, - content: content.map(singleTextCursor), + content: [singleJsonCursor(1)], }; const delta: Delta.Root = new Map([[rootFieldKey, [insert]]]); @@ -814,32 +728,33 @@ export function testForest(config: ForestTestConfiguration): void { ], ], ]); - const expected: JsonableTree[] = [ - { - type: jsonObject.name, - fields: { - y: [ - { - type: jsonNumber.name, - value: 1, - }, - ], - }, - }, - ]; - initializeForest(forest, nestedContent.map(singleTextCursor)); + const expected: JsonCompatible[] = [{ y: 1 }]; + initializeForest(forest, [singleJsonCursor(nestedContent)]); forest.applyDelta(delta); const readCursor = forest.allocateCursor(); moveToDetachedField(forest, readCursor); - const actual = mapCursorField(readCursor, jsonableTreeFromCursor); + const actual = mapCursorField(readCursor, cursorToJsonObject); readCursor.free(); assert.deepEqual(actual, expected); }); it("when moving the last node in the field", () => { + const builder = new SchemaBuilder("moving"); + const leaf = builder.leaf("leaf", ValueSchema.Number); + const root = builder.struct("root", { + x: SchemaBuilder.fieldSequence(leaf), + y: SchemaBuilder.fieldSequence(leaf), + }); + const schema = builder.intoDocumentSchema(SchemaBuilder.fieldOptional(root)); + const forest = factory( - new InMemoryStoredSchemaRepository(defaultSchemaPolicy, jsonSchema), + new InMemoryStoredSchemaRepository(defaultSchemaPolicy, schema), ); - initializeForest(forest, nestedContent.map(singleTextCursor)); + initializeForest(forest, [ + cursorForTypedTreeData({ schema }, root, { + x: [2], + y: [1], + }), + ]); const moveId = brandOpaque(0); const moveOut: Delta.MoveOut = { type: Delta.MarkType.MoveOut, @@ -860,26 +775,11 @@ export function testForest(config: ForestTestConfiguration): void { }; const delta: Delta.Root = new Map([[rootFieldKey, [modify]]]); forest.applyDelta(delta); - const expected: JsonableTree[] = [ - { - type: jsonObject.name, - fields: { - y: [ - { - type: jsonNumber.name, - value: 1, - }, - { - type: jsonNumber.name, - value: 0, - fields: { - foo: [{ type: jsonNumber.name, value: 1 }], - }, - }, - ], - }, - }, - ]; + const expectedCursor = cursorForTypedTreeData({ schema }, root, { + x: [], + y: [1, 2], + }); + const expected: JsonableTree[] = [jsonableTreeFromCursor(expectedCursor)]; const readCursor = forest.allocateCursor(); moveToDetachedField(forest, readCursor); const actual = mapCursorField(readCursor, jsonableTreeFromCursor); From 2395a244ecdf59ca0f1be522c9ae7079c7d8aa01 Mon Sep 17 00:00:00 2001 From: Alex Villarreal <716334+alexvy86@users.noreply.github.com> Date: Tue, 1 Aug 2023 12:41:39 -0500 Subject: [PATCH 5/8] fix(bundle-size): Fix NaNs in bundle size comparison (#16605) --- .../src/ADO/AdoSizeComparator.ts | 18 ++++++------ .../statsProcessors/entryStatsProcessor.ts | 19 +++++++------ .../src/utilities/getChunkParsedSize.ts | 28 +++++++++++++++---- 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/build-tools/packages/bundle-size-tools/src/ADO/AdoSizeComparator.ts b/build-tools/packages/bundle-size-tools/src/ADO/AdoSizeComparator.ts index d9b98fe59a03..96bc81c4f01b 100644 --- a/build-tools/packages/bundle-size-tools/src/ADO/AdoSizeComparator.ts +++ b/build-tools/packages/bundle-size-tools/src/ADO/AdoSizeComparator.ts @@ -106,8 +106,10 @@ export class ADOSizeComparator { if (baselineBuild === undefined) { baselineCommit = fallbackGen?.next().value; + // For reasons that I don't understand, the "undefined" string is omitted in the log output, which makes the + // output very confusing. The string is capitalized here and elsewhere in this file as a workaround. console.log( - `Trying backup baseline commit when baseline build is undefined ${baselineCommit}`, + `Trying backup baseline commit when baseline build is UNDEFINED: ${baselineCommit}`, ); continue; } @@ -162,7 +164,9 @@ export class ADOSizeComparator { return undefined; }); - console.log(`Baseline Zip === undefined: ${baselineZip === undefined}`); + // For reasons that I don't understand, the "undefined" string is omitted in the log output, which makes the + // output very confusing. The string is capitalized here and elsewhere in this file as a workaround. + console.log(`Baseline Zip === UNDEFINED: ${baselineZip === undefined}`); // Successful baseline build does not have the needed build artifacts if (baselineZip === undefined) { @@ -184,10 +188,7 @@ export class ADOSizeComparator { return { message, comparison: undefined }; } - const comparison: BundleComparison[] = await this.createComparisonFromZip( - baselineCommit, - baselineZip, - ); + const comparison: BundleComparison[] = await this.createComparisonFromZip(baselineZip); console.log(JSON.stringify(comparison)); const message = getCommentForBundleDiff(comparison, baselineCommit); @@ -212,10 +213,7 @@ export class ADOSizeComparator { } } - private async createComparisonFromZip( - baselineCommit: string, - baselineZip: JSZip, - ): Promise { + private async createComparisonFromZip(baselineZip: JSZip): Promise { const baselineZipBundlePaths = getBundlePathsFromZipObject(baselineZip); const prBundleFileSystemPaths = await getBundlePathsFromFileSystem(this.localReportPath); diff --git a/build-tools/packages/bundle-size-tools/src/statsProcessors/entryStatsProcessor.ts b/build-tools/packages/bundle-size-tools/src/statsProcessors/entryStatsProcessor.ts index 8dd536dc87a8..76a2b68dcda8 100644 --- a/build-tools/packages/bundle-size-tools/src/statsProcessors/entryStatsProcessor.ts +++ b/build-tools/packages/bundle-size-tools/src/statsProcessors/entryStatsProcessor.ts @@ -2,10 +2,7 @@ * Copyright (c) Microsoft Corporation and contributors. All rights reserved. * Licensed under the MIT License. */ -import { fail } from "assert"; - import { BundleMetric, WebpackStatsProcessor } from "../BundleBuddyTypes"; -import { getChunkParsedSize } from "../utilities"; export interface EntryStatsProcessorOptions { // Custom callback to customize what text will be used as the metric name @@ -13,7 +10,7 @@ export interface EntryStatsProcessorOptions { } /** - * A simple stats processor that simply returns the size information for the entry chunk + * Returns a stats processor that returns total asset size information for each entryPoint in the stats object */ export function getEntryStatsProcessor(options: EntryStatsProcessorOptions): WebpackStatsProcessor { return (stats) => { @@ -28,11 +25,17 @@ export function getEntryStatsProcessor(options: EntryStatsProcessorOptions): Web const metricName = options.metricNameProvider ? options.metricNameProvider(chunkName) : chunkName; + + // Note: we have the getChunkParsedSize function, but the entrypoints objects we're analyzing here already + // have a list of the relevant assets and their sizes; no need to take the entrypoints' chunks and pass them to + // that function. + let totalSize: number = 0; + for (const asset of chunkGroupStats.assets ?? []) { + totalSize += asset?.size ?? 0; + } + result.set(metricName, { - parsedSize: getChunkParsedSize( - stats, - (chunkGroupStats.chunks ?? fail("missing chunk"))[0], - ), + parsedSize: totalSize, }); }); diff --git a/build-tools/packages/bundle-size-tools/src/utilities/getChunkParsedSize.ts b/build-tools/packages/bundle-size-tools/src/utilities/getChunkParsedSize.ts index 717cdd4e7a1f..9f68d1191bfc 100644 --- a/build-tools/packages/bundle-size-tools/src/utilities/getChunkParsedSize.ts +++ b/build-tools/packages/bundle-size-tools/src/utilities/getChunkParsedSize.ts @@ -14,17 +14,33 @@ export function getChunkParsedSize(stats: StatsCompilation, chunkId: string | nu ); } - const matchingAsset = stats.assets.find((asset) => { + //Find all the assets that contain the chunk. Note: an asset may contain more than one chunk. + const matchingAssets = stats.assets.filter((asset) => { // Make sure to only look at js files and not source maps (assumes source maps don't end in .js) if (asset.name.endsWith(".js")) { - // Assumes only a single chunk per asset, this may not hold for all apps. - return asset.chunks?.[0] === chunkId; + // If the asset contains the chunk, it should be considered when calculating the total size. + return asset.chunks?.includes(chunkId); } return false; }); - // If there's no matching asset it could be that it was removed in the new version of the bundle, not necessarily an - // error. In that case return 0 as its size. - return matchingAsset?.size ?? 0; + if (matchingAssets.length === 0) { + throw new Error( + `Could not find an asset for chunk with id '${chunkId}' in the webpack stats`, + ); + } + + if (matchingAssets.length > 1) { + // Typically we expect a single asset to be found per chunk (this is maybe not typical of all webpack projects, but + // it seems to be the case in our usage here), so if we find more than one, log a warning so we can investigate more + // easily if needed. + console.warn( + `${matchingAssets.length} assets contain chunk with id '${chunkId}'; will return total size of all matching assets.`, + ); + } + + // The total size is the sum of the sizes of all assets with the chunk. + const totalSize = matchingAssets.reduce((acc, asset) => acc + asset.size, 0); + return totalSize; } From 34a110c00596fd038e09df1ae3ff3c66859d9015 Mon Sep 17 00:00:00 2001 From: t-santiagode <134635443+t-santiagode@users.noreply.github.com> Date: Tue, 1 Aug 2023 11:19:22 -0700 Subject: [PATCH 6/8] feat(devtools): Add basic editing skeleton (#16366) Description Made the changes reuqired to edit SharedCounter and SharedString. This included: - DataEditing messages and handlers to communicate the intent to edit a DDS - Infrastructure once the message is received to user the data in a message and properly apply it to the indicated DDS - UI which surfaces the ability to make changes to the DDS - Feature Flags for container-scoped features (Editing is disabled by default, and visualizers are still enabled by default) --------- Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> Co-authored-by: Joshua Smithrud Co-authored-by: Ji Kim --- .../api-report/devtools-core.api.md | 43 +++- .../tools/devtools/devtools-core/package.json | 4 + .../devtools-core/src/CommonInterfaces.ts | 12 ++ .../devtools-core/src/ContainerDevtools.ts | 26 ++- .../devtools/devtools-core/src/Features.ts | 8 +- .../src/data-visualization/DataEditing.ts | 41 ++++ .../data-visualization/DataVisualization.ts | 52 +++++ .../src/data-visualization/DefaultEditors.ts | 54 ++++++ .../data-visualization/DefaultVisualizers.ts | 3 + .../src/data-visualization/VisualTree.ts | 8 +- .../src/data-visualization/index.ts | 2 + .../tools/devtools/devtools-core/src/index.ts | 12 +- .../container-devtools-messages/DataEdit.ts | 60 ++++++ .../container-devtools-messages/index.ts | 1 + .../devtools-core/src/messaging/index.ts | 1 + .../src/test/DataVisualizerGraph.test.ts | 8 + .../src/test/DefaultVisualizers.test.ts | 4 +- .../validateDevtoolsCorePrevious.generated.ts | 1 + .../tools/devtools/devtools-view/package.json | 1 + .../src/ContainerFeatureFlagHelper.ts | 34 ++++ .../src/components/ContainerDevtoolsView.tsx | 10 +- .../data-visualization/EditableValueView.tsx | 183 ++++++++++++++++++ .../data-visualization/FluidHandleView.tsx | 10 +- .../data-visualization/FluidValueView.tsx | 40 ++-- .../data-visualization/TreeDataView.tsx | 2 +- .../data-visualization/TreeItem.tsx | 1 + pnpm-lock.yaml | 4 +- 27 files changed, 601 insertions(+), 24 deletions(-) create mode 100644 packages/tools/devtools/devtools-core/src/data-visualization/DataEditing.ts create mode 100644 packages/tools/devtools/devtools-core/src/data-visualization/DefaultEditors.ts create mode 100644 packages/tools/devtools/devtools-core/src/messaging/container-devtools-messages/DataEdit.ts create mode 100644 packages/tools/devtools/devtools-view/src/ContainerFeatureFlagHelper.ts create mode 100644 packages/tools/devtools/devtools-view/src/components/data-visualization/EditableValueView.tsx diff --git a/packages/tools/devtools/devtools-core/api-report/devtools-core.api.md b/packages/tools/devtools/devtools-core/api-report/devtools-core.api.md index 57e483d331d3..3a2299593f5b 100644 --- a/packages/tools/devtools/devtools-core/api-report/devtools-core.api.md +++ b/packages/tools/devtools/devtools-core/api-report/devtools-core.api.md @@ -12,8 +12,10 @@ import { IDisposable } from '@fluidframework/core-interfaces'; import { IEvent } from '@fluidframework/common-definitions'; import { IEventProvider } from '@fluidframework/common-definitions'; import { IFluidLoadable } from '@fluidframework/core-interfaces'; +import { ISharedObject } from '@fluidframework/shared-object-base'; import { ITelemetryBaseEvent } from '@fluidframework/core-interfaces'; import { ITelemetryBaseLogger } from '@fluidframework/core-interfaces'; +import { Serializable } from '@fluidframework/datastore-definitions'; // @internal export interface AudienceChangeLogEntry extends LogEntry { @@ -69,7 +71,8 @@ export interface ConnectionStateChangeLogEntry extends StateChangeLogEntry { + type: typeof MessageType; + } + export interface MessageData extends HasContainerKey { + edit: SharedObjectEdit; + } +} + // @internal export namespace DataVisualization { const MessageType = "DATA_VISUALIZATION"; @@ -218,6 +233,25 @@ export namespace DisconnectContainer { export type MessageData = HasContainerKey; } +// @internal +export interface Edit { + data: Serializable; + type?: EditType | string; +} + +// @internal +export type EditSharedObject = (sharedObject: ISharedObject, edit: Edit) => Promise; + +// @internal +export enum EditType { + // (undocumented) + Boolean = "boolean", + // (undocumented) + Number = "number", + // (undocumented) + String = "string" +} + // @public export interface FluidDevtoolsProps { initialContainers?: ContainerDevtoolsProps[]; @@ -427,6 +461,10 @@ export namespace RootDataVisualizations { // @internal export type RootHandleNode = FluidHandleNode | UnknownObjectNode; +// @internal +export interface SharedObjectEdit extends Edit, HasFluidObjectId { +} + // @internal export interface StateChangeLogEntry extends LogEntry { newState: TState; @@ -479,6 +517,9 @@ export type VisualNode = VisualTreeNode | VisualValueNode | FluidHandleNode | Fl // @internal export interface VisualNodeBase { + editProps?: { + editTypes?: EditType[]; + }; metadata?: Record; nodeKind: VisualNodeKind | string; typeMetadata?: string; diff --git a/packages/tools/devtools/devtools-core/package.json b/packages/tools/devtools/devtools-core/package.json index b8b702edb669..976b473f3649 100644 --- a/packages/tools/devtools/devtools-core/package.json +++ b/packages/tools/devtools/devtools-core/package.json @@ -71,6 +71,7 @@ "@fluidframework/container-utils": "workspace:~", "@fluidframework/core-interfaces": "workspace:~", "@fluidframework/counter": "workspace:~", + "@fluidframework/datastore-definitions": "workspace:~", "@fluidframework/map": "workspace:~", "@fluidframework/matrix": "workspace:~", "@fluidframework/protocol-definitions": "^1.1.0", @@ -118,6 +119,9 @@ }, "typeValidation": { "broken": { + "EnumDeclaration_ContainerDevtoolsFeature": { + "backCompat": false + }, "ClassDeclaration_DevtoolsLogger": { "backCompat": false }, diff --git a/packages/tools/devtools/devtools-core/src/CommonInterfaces.ts b/packages/tools/devtools/devtools-core/src/CommonInterfaces.ts index 747f40d637e6..608d5f85ab33 100644 --- a/packages/tools/devtools/devtools-core/src/CommonInterfaces.ts +++ b/packages/tools/devtools/devtools-core/src/CommonInterfaces.ts @@ -45,3 +45,15 @@ export interface HasFluidObjectId { */ fluidObjectId: FluidObjectId; } + +/** + * Base interface used in message data for edits allowed by a particular Fluid object (DDS) via + * an enum. + * + * @internal + */ +export enum EditType { + Number = "number", + String = "string", + Boolean = "boolean", +} diff --git a/packages/tools/devtools/devtools-core/src/ContainerDevtools.ts b/packages/tools/devtools/devtools-core/src/ContainerDevtools.ts index eb8d11a31188..342a2e2048a7 100644 --- a/packages/tools/devtools/devtools-core/src/ContainerDevtools.ts +++ b/packages/tools/devtools/devtools-core/src/ContainerDevtools.ts @@ -13,8 +13,10 @@ import { ContainerStateMetadata } from "./ContainerMetadata"; import { DataVisualizerGraph, defaultVisualizers, + defaultEditors, FluidObjectNode, RootHandleNode, + SharedObjectEdit, } from "./data-visualization"; import { IContainerDevtools } from "./IContainerDevtools"; import { AudienceChangeLogEntry, ConnectionStateChangeLogEntry } from "./Logs"; @@ -25,6 +27,7 @@ import { ContainerDevtoolsFeatures, ContainerStateChange, ContainerStateHistory, + DataEdit, DataVisualization, DisconnectContainer, GetAudienceSummary, @@ -325,6 +328,15 @@ export class ContainerDevtools implements IContainerDevtools, HasContainerKey { } return false; }, + + [DataEdit.MessageType]: async (untypedMessage) => { + const message = untypedMessage as DataEdit.Message; + if (message.data.containerKey === this.containerKey) { + await this.editData(message.data.edit); + return true; + } + return false; + }, }; /** @@ -447,7 +459,8 @@ export class ContainerDevtools implements IContainerDevtools, HasContainerKey { this.dataVisualizer = props.containerData === undefined ? undefined - : new DataVisualizerGraph(props.containerData, defaultVisualizers); + : new DataVisualizerGraph(props.containerData, defaultVisualizers, defaultEditors); + this.dataVisualizer?.on("update", this.dataUpdateHandler); // Bind Container events required for change-logging @@ -522,6 +535,10 @@ export class ContainerDevtools implements IContainerDevtools, HasContainerKey { private getSupportedFeatures(): ContainerDevtoolsFeatureFlags { return { [ContainerDevtoolsFeature.ContainerData]: this.containerData !== undefined, + /** + * Todo: When ready to enable feature set it to this.containerData !== undefined + */ + [ContainerDevtoolsFeature.ContainerDataEditing]: false, }; } @@ -549,4 +566,11 @@ export class ContainerDevtools implements IContainerDevtools, HasContainerKey { ): Promise { return this.dataVisualizer?.render(fluidObjectId) ?? undefined; } + + /** + * Applies an {@link Edit} to a {@link SharedObject} + */ + private async editData(edit: SharedObjectEdit): Promise { + return this.dataVisualizer?.applyEdit(edit); + } } diff --git a/packages/tools/devtools/devtools-core/src/Features.ts b/packages/tools/devtools/devtools-core/src/Features.ts index 69141dbd0de2..8c9eab059cae 100644 --- a/packages/tools/devtools/devtools-core/src/Features.ts +++ b/packages/tools/devtools/devtools-core/src/Features.ts @@ -45,10 +45,14 @@ export type DevtoolsFeatureFlags = { */ export enum ContainerDevtoolsFeature { /** - * Indicates that the Container Devtools is capable of generating visual summaries of application data associated with - * the Container. + * Indicates that the Container Devtools supports editing the data associated with the Container. */ ContainerData = "container-data", + + /** + * Indicates that editing of values is available in the Devtools View. + */ + ContainerDataEditing = "container-data-editing", } /** diff --git a/packages/tools/devtools/devtools-core/src/data-visualization/DataEditing.ts b/packages/tools/devtools/devtools-core/src/data-visualization/DataEditing.ts new file mode 100644 index 000000000000..cab3f91c535e --- /dev/null +++ b/packages/tools/devtools/devtools-core/src/data-visualization/DataEditing.ts @@ -0,0 +1,41 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { ISharedObject } from "@fluidframework/shared-object-base"; +import { Serializable } from "@fluidframework/datastore-definitions"; +import { EditType, HasFluidObjectId } from "../CommonInterfaces"; + +/** + * Applies an edit to {@link @fluidframework/shared-object-base#ISharedObject}. + * @param sharedObject - The {@link @fluidframework/shared-object-base#ISharedObject} whose data will be edited. + * @param edit - Describes what changes will be made using {@link Edit}. + * + * @internal + */ +export type EditSharedObject = (sharedObject: ISharedObject, edit: Edit) => Promise; + +/** + * Interface to contain information necesary for an edit + * @internal + */ +export interface Edit { + /** + * Type contains the {@link EditType} of the edit being preformed. + * + * @remarks This is generally expected to be of type `EditType`. `string` is supported strictly for forward / backward compatibility. If "type" is undefined then it assumes the type of data. + */ + type?: EditType | string; + + /** + * Data contains the new data that will be edited into the DDS + */ + data: Serializable; +} + +/** + * Interface to contain information necesary for an edit of a SharedObject + * @internal + */ +export interface SharedObjectEdit extends Edit, HasFluidObjectId {} diff --git a/packages/tools/devtools/devtools-core/src/data-visualization/DataVisualization.ts b/packages/tools/devtools/devtools-core/src/data-visualization/DataVisualization.ts index 81bfc8e211ab..de77a1f60036 100644 --- a/packages/tools/devtools/devtools-core/src/data-visualization/DataVisualization.ts +++ b/packages/tools/devtools/devtools-core/src/data-visualization/DataVisualization.ts @@ -24,6 +24,7 @@ import { RootHandleNode, unknownObjectNode, } from "./VisualTree"; +import { Edit, EditSharedObject, SharedObjectEdit } from "./DataEditing"; // Ideas: // - Hold onto previous summary and only transmit diff? @@ -94,6 +95,23 @@ export interface SharedObjectVisualizers { [k: SharedObjectType]: VisualizeSharedObject; } +/** + * Specifies editors for different {@link @fluidframework/shared-object-base#ISharedObject} types. + * + * @remarks + * + * - `key`: The type of Shared object ({@link @fluidframework/datastore-definitions#IChannelFactory.Type}). + * + * - `value`: A editor that takes a {@link @fluidframework/shared-object-base#ISharedObject} of the + * specified type and preforms the corresponding edit for it. + */ +export interface SharedObjectEditors { + /** + * Individual Fluid object editors, keyed by {@link SharedObjectType}. + */ + [k: SharedObjectType]: EditSharedObject; +} + /** * Data visualization update events. */ @@ -155,6 +173,11 @@ export class DataVisualizerGraph * Policy object for visualizing different kinds of shared objects. */ private readonly visualizers: SharedObjectVisualizers, + + /** + * Policy object for editing different kinds of shared objects. + */ + private readonly editors: SharedObjectEditors, ) { super(); @@ -210,6 +233,15 @@ export class DataVisualizerGraph return this.visualizerNodes.get(fluidObjectId)?.render() ?? undefined; } + /** + * Applies an edit to a Fluid object. + * @param edit - is a Edit object that describes an edit to a Fluid object. + * @returns - A promise that resolves when the editing of a {@link @fluidframework/shared-object-base#ISharedObject} is complete + */ + public async applyEdit(edit: SharedObjectEdit): Promise { + return this.visualizerNodes.get(edit.fluidObjectId)?.applyEdit(edit); + } + /** * Adds a visualizer node to the collection for the specified * {@link @fluidframework/shared-object-base#ISharedObject} if one does not already exist. @@ -221,9 +253,14 @@ export class DataVisualizerGraph this.visualizers[sharedObject.attributes.type] !== undefined ? this.visualizers[sharedObject.attributes.type] : visualizeUnknownSharedObject; + + // Create visualizer node for the shared object + const editorFunction = this.editors[sharedObject.attributes.type]; + const visualizerNode = new VisualizerNode( sharedObject, visualizationFunction, + editorFunction, async (handle) => this.registerVisualizerForHandle(handle), ); @@ -330,6 +367,12 @@ export class VisualizerNode extends TypedEventEmitter impl */ private readonly visualizeSharedObject: VisualizeSharedObject, + /** + * Callback for editing {@link VisualizerNode.sharedObject}. + * Encapsulates the policies for editing different kinds of DDSs. + */ + private readonly editSharedObject: EditSharedObject, + /** * Registers some child handle to a Fluid object for future rendering. * @@ -388,6 +431,15 @@ export class VisualizerNode extends TypedEventEmitter impl ); } + /** + * Edits a {@link @fluidframework/shared-object-base#ISharedObject} + * @param edit - Describes an edit to a Fluid object. + * @returns - A promise that resolves when the editing of a {@link @fluidframework/shared-object-base#ISharedObject} is complete + */ + public async applyEdit(edit: Edit): Promise { + return this.editSharedObject(this.sharedObject, edit); + } + /** * {@inheritDoc VisualizeChildData} */ diff --git a/packages/tools/devtools/devtools-core/src/data-visualization/DefaultEditors.ts b/packages/tools/devtools/devtools-core/src/data-visualization/DefaultEditors.ts new file mode 100644 index 000000000000..e6f29de7a85f --- /dev/null +++ b/packages/tools/devtools/devtools-core/src/data-visualization/DefaultEditors.ts @@ -0,0 +1,54 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +/** + * This module contains default {@link EditSharedObject} + * implementations for our DDSs. + */ + +import { SharedCounter } from "@fluidframework/counter"; +import { SharedString } from "@fluidframework/sequence"; + +import { ISharedObject } from "@fluidframework/shared-object-base"; +import { Edit, EditSharedObject } from "./DataEditing"; + +/** + * Default {@link EditSharedObject} for {@link SharedCounter}. + */ +export const editSharedCounter: EditSharedObject = async ( + sharedObject: ISharedObject, + edit: Edit, +): Promise => { + console.log("testing editSharedCounter"); + if (typeof edit.data !== "number") return; + const sharedCounter = sharedObject as SharedCounter; + sharedCounter.increment(Math.floor(edit.data) - sharedCounter.value); +}; + +/** + * Default {@link EditSharedObject} for {@link SharedString}. + */ +export const editSharedString: EditSharedObject = async ( + sharedObject: ISharedObject, + edit: Edit, +): Promise => { + if (typeof edit.data !== "string") return; + const sharedString = sharedObject as SharedString; + if (edit.data === "") { + sharedString.removeText(0, sharedString.getLength()); + } else { + sharedString.replaceText(0, sharedString.getLength(), edit.data); + } +}; + +/** + * List of default editors included in the library. + */ +export const defaultEditors: Record = { + [SharedCounter.getFactory().type]: editSharedCounter, + [SharedString.getFactory().type]: editSharedString, + + // TODO: the others +}; diff --git a/packages/tools/devtools/devtools-core/src/data-visualization/DefaultVisualizers.ts b/packages/tools/devtools/devtools-core/src/data-visualization/DefaultVisualizers.ts index 4541009f5eb1..54f5802e7456 100644 --- a/packages/tools/devtools/devtools-core/src/data-visualization/DefaultVisualizers.ts +++ b/packages/tools/devtools/devtools-core/src/data-visualization/DefaultVisualizers.ts @@ -20,6 +20,7 @@ import { UntypedField, } from "@fluid-experimental/tree2"; import { ISharedObject } from "@fluidframework/shared-object-base"; +import { EditType } from "../CommonInterfaces"; import { VisualizeChildData, VisualizeSharedObject } from "./DataVisualization"; import { FluidObjectTreeNode, @@ -62,6 +63,7 @@ export const visualizeSharedCounter: VisualizeSharedObject = async ( value: sharedCounter.value, typeMetadata: "SharedCounter", nodeKind: VisualNodeKind.FluidValueNode, + editProps: { editTypes: [EditType.Number] }, }; }; @@ -192,6 +194,7 @@ export const visualizeSharedString: VisualizeSharedObject = async ( value: text, typeMetadata: "SharedString", nodeKind: VisualNodeKind.FluidValueNode, + editProps: { editTypes: [EditType.String] }, }; }; diff --git a/packages/tools/devtools/devtools-core/src/data-visualization/VisualTree.ts b/packages/tools/devtools/devtools-core/src/data-visualization/VisualTree.ts index 3f7f70487f1f..668f96fca5a6 100644 --- a/packages/tools/devtools/devtools-core/src/data-visualization/VisualTree.ts +++ b/packages/tools/devtools/devtools-core/src/data-visualization/VisualTree.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { FluidObjectId } from "../CommonInterfaces"; +import { FluidObjectId, EditType } from "../CommonInterfaces"; /** * This module contains a type system for describing visual descriptors of data objects in a serializable @@ -68,6 +68,12 @@ export interface VisualNodeBase { * Consumers of this value should attempt to handle unrecognized values gracefully. */ nodeKind: VisualNodeKind | string; + + /** + * (optional) If editProps is present it indicates the node is editable. + * Inside of the porperty is an array of possible EditTypes to inform devtools-view to only show the corresponding edit options for the types allowed + */ + editProps?: { editTypes?: EditType[] }; } /** diff --git a/packages/tools/devtools/devtools-core/src/data-visualization/index.ts b/packages/tools/devtools/devtools-core/src/data-visualization/index.ts index 680afa722144..59702c3abc8a 100644 --- a/packages/tools/devtools/devtools-core/src/data-visualization/index.ts +++ b/packages/tools/devtools/devtools-core/src/data-visualization/index.ts @@ -13,6 +13,8 @@ export { visualizeChildData, VisualizerNode, } from "./DataVisualization"; +export { Edit, EditSharedObject, SharedObjectEdit } from "./DataEditing"; +export { defaultEditors } from "./DefaultEditors"; export { defaultVisualizers, visualizeSharedCell, diff --git a/packages/tools/devtools/devtools-core/src/index.ts b/packages/tools/devtools/devtools-core/src/index.ts index b9ae81058785..7593a13627b3 100644 --- a/packages/tools/devtools/devtools-core/src/index.ts +++ b/packages/tools/devtools/devtools-core/src/index.ts @@ -22,11 +22,19 @@ */ export { AudienceClientMetadata, MemberChangeKind } from "./AudienceMetadata"; -export { ContainerKey, FluidObjectId, HasContainerKey, HasFluidObjectId } from "./CommonInterfaces"; +export { + ContainerKey, + EditType, + FluidObjectId, + HasContainerKey, + HasFluidObjectId, +} from "./CommonInterfaces"; export { ContainerStateChangeKind } from "./Container"; export { ContainerDevtoolsProps } from "./ContainerDevtools"; export { ContainerStateMetadata } from "./ContainerMetadata"; export { + Edit, + EditSharedObject, FluidHandleNode, FluidObjectNode, FluidObjectNodeBase, @@ -35,6 +43,7 @@ export { FluidUnknownObjectNode, Primitive, RootHandleNode, + SharedObjectEdit, TreeNodeBase, ValueNodeBase, VisualChildNode, @@ -68,6 +77,7 @@ export { ContainerList, ContainerStateChange, ContainerStateHistory, + DataEdit, DataVisualization, DevtoolsDisposed, DevtoolsFeatures, diff --git a/packages/tools/devtools/devtools-core/src/messaging/container-devtools-messages/DataEdit.ts b/packages/tools/devtools/devtools-core/src/messaging/container-devtools-messages/DataEdit.ts new file mode 100644 index 000000000000..59c09b33b3ac --- /dev/null +++ b/packages/tools/devtools/devtools-core/src/messaging/container-devtools-messages/DataEdit.ts @@ -0,0 +1,60 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { HasContainerKey } from "../../CommonInterfaces"; +import { SharedObjectEdit } from "../../data-visualization"; +import { IDevtoolsMessage } from "../Messages"; + +/** + * Encapsulates types and logic related to {@link DataEdit.Message}. + * + * @internal + */ +export namespace DataEdit { + /** + * {@link DataEdit.Message} {@link IDevtoolsMessage."type"}. + * + * @internal + */ + export const MessageType = "DATA_EDIT"; + + /** + * Message data format used by {@link DataEdit.Message}. + * + * @internal + */ + export interface MessageData extends HasContainerKey { + /** + * edit includes a {@link SharedObjectEdit} which constains the information necesary to preform an edit + */ + edit: SharedObjectEdit; + } + + /** + * Inbound message for editing a specific DDS via its associated {@link HasFluidObjectId.fluidObjectId}. + * + * Will result in the DDS being edited. + * + * @internal + */ + export interface Message extends IDevtoolsMessage { + /** + * {@inheritDoc IDevtoolsMessage."type"} + */ + type: typeof MessageType; + } + + /** + * Creates a {@link DataEdit.Message} from the provided {@link DataEdit.MessageData}. + * + * @internal + */ + export function createMessage(data: MessageData): Message { + return { + data, + type: MessageType, + }; + } +} diff --git a/packages/tools/devtools/devtools-core/src/messaging/container-devtools-messages/index.ts b/packages/tools/devtools/devtools-core/src/messaging/container-devtools-messages/index.ts index 269e0505605b..1313a33bb876 100644 --- a/packages/tools/devtools/devtools-core/src/messaging/container-devtools-messages/index.ts +++ b/packages/tools/devtools/devtools-core/src/messaging/container-devtools-messages/index.ts @@ -13,6 +13,7 @@ export { ConnectContainer } from "./ConnectContainer"; export { ContainerDevtoolsFeatures } from "./ContainerDevtoolsFeatures"; export { ContainerStateChange } from "./ContainerStateChange"; export { ContainerStateHistory } from "./ContainerStateHistory"; +export { DataEdit } from "./DataEdit"; export { DataVisualization } from "./DataVisualization"; export { DisconnectContainer } from "./DisconnectContainer"; export { GetAudienceSummary } from "./GetAudienceSummary"; diff --git a/packages/tools/devtools/devtools-core/src/messaging/index.ts b/packages/tools/devtools/devtools-core/src/messaging/index.ts index dabe668effb3..4262f2286adc 100644 --- a/packages/tools/devtools/devtools-core/src/messaging/index.ts +++ b/packages/tools/devtools/devtools-core/src/messaging/index.ts @@ -21,6 +21,7 @@ export { ContainerDevtoolsFeatures, ContainerStateChange, ContainerStateHistory, + DataEdit, DataVisualization, DisconnectContainer, GetAudienceSummary, diff --git a/packages/tools/devtools/devtools-core/src/test/DataVisualizerGraph.test.ts b/packages/tools/devtools/devtools-core/src/test/DataVisualizerGraph.test.ts index 6cbf502f24cd..3415ea574f0e 100644 --- a/packages/tools/devtools/devtools-core/src/test/DataVisualizerGraph.test.ts +++ b/packages/tools/devtools/devtools-core/src/test/DataVisualizerGraph.test.ts @@ -17,7 +17,9 @@ import { FluidObjectTreeNode, FluidObjectValueNode, VisualNodeKind, + defaultEditors, } from "../data-visualization"; +import { EditType } from "../CommonInterfaces"; describe("DataVisualizerGraph unit tests", () => { it("Single root DDS (SharedCounter)", async () => { @@ -33,6 +35,7 @@ describe("DataVisualizerGraph unit tests", () => { counter: sharedCounter, }, defaultVisualizers, + defaultEditors, ); const rootTrees = await visualizer.renderRootHandles(); @@ -46,6 +49,7 @@ describe("DataVisualizerGraph unit tests", () => { value: 0, typeMetadata: "SharedCounter", nodeKind: VisualNodeKind.FluidValueNode, + editProps: { editTypes: [EditType.Number] }, }; expect(childTree).to.deep.equal(expectedChildTree); @@ -59,6 +63,7 @@ describe("DataVisualizerGraph unit tests", () => { value: 37, typeMetadata: "SharedCounter", nodeKind: VisualNodeKind.FluidValueNode, + editProps: { editTypes: [EditType.Number] }, }; expect(childTreeAfterEdit).to.deep.equal(expectedChildTreeAfterEdit); }); @@ -74,6 +79,7 @@ describe("DataVisualizerGraph unit tests", () => { map: sharedMap, }, defaultVisualizers, + defaultEditors, ); const rootTrees = await visualizer.renderRootHandles(); @@ -166,6 +172,7 @@ describe("DataVisualizerGraph unit tests", () => { cell: sharedCell, }, defaultVisualizers, + defaultEditors, ); const rootTrees = await visualizer.renderRootHandles(); @@ -182,6 +189,7 @@ describe("DataVisualizerGraph unit tests", () => { value: 42, typeMetadata: "SharedCounter", nodeKind: VisualNodeKind.FluidValueNode, + editProps: { editTypes: [EditType.Number] }, }; expect(childCounterTree).to.deep.equal(expectedChildCounterTree); diff --git a/packages/tools/devtools/devtools-core/src/test/DefaultVisualizers.test.ts b/packages/tools/devtools/devtools-core/src/test/DefaultVisualizers.test.ts index 76a7e9b054c9..5df86e705112 100644 --- a/packages/tools/devtools/devtools-core/src/test/DefaultVisualizers.test.ts +++ b/packages/tools/devtools/devtools-core/src/test/DefaultVisualizers.test.ts @@ -25,7 +25,7 @@ import { valueSymbol, } from "@fluid-experimental/tree2"; -import { FluidObjectId } from "../CommonInterfaces"; +import { EditType, FluidObjectId } from "../CommonInterfaces"; import { FluidObjectTreeNode, FluidObjectValueNode, @@ -94,6 +94,7 @@ describe("DefaultVisualizers unit tests", () => { value: 37, typeMetadata: "SharedCounter", nodeKind: VisualNodeKind.FluidValueNode, + editProps: { editTypes: [EditType.Number] }, }; expect(result).to.deep.equal(expected); @@ -371,6 +372,7 @@ describe("DefaultVisualizers unit tests", () => { value: "Hello World!", typeMetadata: "SharedString", nodeKind: VisualNodeKind.FluidValueNode, + editProps: { editTypes: [EditType.String] }, }; expect(result).to.deep.equal(expected); diff --git a/packages/tools/devtools/devtools-core/src/test/types/validateDevtoolsCorePrevious.generated.ts b/packages/tools/devtools/devtools-core/src/test/types/validateDevtoolsCorePrevious.generated.ts index c4abd36f736e..61c471dcb185 100644 --- a/packages/tools/devtools/devtools-core/src/test/types/validateDevtoolsCorePrevious.generated.ts +++ b/packages/tools/devtools/devtools-core/src/test/types/validateDevtoolsCorePrevious.generated.ts @@ -395,6 +395,7 @@ declare function get_current_EnumDeclaration_ContainerDevtoolsFeature(): declare function use_old_EnumDeclaration_ContainerDevtoolsFeature( use: TypeOnly); use_old_EnumDeclaration_ContainerDevtoolsFeature( + // @ts-expect-error compatibility expected to be broken get_current_EnumDeclaration_ContainerDevtoolsFeature()); /* diff --git a/packages/tools/devtools/devtools-view/package.json b/packages/tools/devtools/devtools-view/package.json index 31b578796aa9..ae5ac9d536f0 100644 --- a/packages/tools/devtools/devtools-view/package.json +++ b/packages/tools/devtools/devtools-view/package.json @@ -45,6 +45,7 @@ "@fluidframework/common-utils": "^1.1.1", "@fluidframework/container-definitions": "workspace:~", "@fluidframework/container-loader": "workspace:~", + "@fluidframework/datastore-definitions": "workspace:~", "@fluidframework/driver-definitions": "workspace:~", "@fluidframework/protocol-definitions": "^1.1.0", "@fluidframework/telemetry-utils": "workspace:~", diff --git a/packages/tools/devtools/devtools-view/src/ContainerFeatureFlagHelper.ts b/packages/tools/devtools/devtools-view/src/ContainerFeatureFlagHelper.ts new file mode 100644 index 000000000000..28bfa851377d --- /dev/null +++ b/packages/tools/devtools/devtools-view/src/ContainerFeatureFlagHelper.ts @@ -0,0 +1,34 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ +import { ContainerDevtoolsFeatureFlags } from "@fluid-experimental/devtools-core"; +import React from "react"; + +/** + * Interface for {@link ContainerFeatureFlagContext} data + */ +export interface ContainerFeatureFlagContextData { + /** + * {@inheritDoc @fluid-experimental/devtools-core#ContainerDevtoolsFeatureFlags} + */ + containerFeatureFlags: ContainerDevtoolsFeatureFlags; +} + +/** + * Creates the context for container feature flags + */ +export const ContainerFeatureFlagContext = React.createContext< + ContainerFeatureFlagContextData | undefined +>(undefined); + +/** + * Used to get the context or throw an Error if not found + */ +export function useContainerFeaturesContext(): ContainerFeatureFlagContextData { + const context = React.useContext(ContainerFeatureFlagContext); + if (context === undefined) { + throw new Error("TODO"); + } + return context; +} diff --git a/packages/tools/devtools/devtools-view/src/components/ContainerDevtoolsView.tsx b/packages/tools/devtools/devtools-view/src/components/ContainerDevtoolsView.tsx index 72c51b801e3d..129284674e38 100644 --- a/packages/tools/devtools/devtools-view/src/components/ContainerDevtoolsView.tsx +++ b/packages/tools/devtools/devtools-view/src/components/ContainerDevtoolsView.tsx @@ -26,6 +26,7 @@ import { import React from "react"; import { useMessageRelay } from "../MessageRelayContext"; +import { ContainerFeatureFlagContext } from "../ContainerFeatureFlagHelper"; import { useLogger } from "../TelemetryUtils"; import { AudienceView } from "./AudienceView"; import { ContainerHistoryView } from "./ContainerHistoryView"; @@ -101,6 +102,7 @@ export function ContainerDevtoolsView(props: ContainerDevtoolsViewProps): React. const message = untypedMessage as ContainerDevtoolsFeatures.Message; if (message.data.containerKey === containerKey) { setSupportedFeatures(message.data.features); + return true; } return false; @@ -162,7 +164,13 @@ function _ContainerDevtoolsView(props: _ContainerDevtoolsViewProps): React.React let innerView: React.ReactElement; switch (innerViewSelection) { case PanelView.ContainerData: - innerView = ; + innerView = ( + + + + ); break; case PanelView.Audience: innerView = ; diff --git a/packages/tools/devtools/devtools-view/src/components/data-visualization/EditableValueView.tsx b/packages/tools/devtools/devtools-view/src/components/data-visualization/EditableValueView.tsx new file mode 100644 index 000000000000..4ae804511d7d --- /dev/null +++ b/packages/tools/devtools/devtools-view/src/components/data-visualization/EditableValueView.tsx @@ -0,0 +1,183 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import React from "react"; + +import { Input, InputOnChangeData } from "@fluentui/react-components"; +import { FluidObjectValueNode, DataEdit } from "@fluid-experimental/devtools-core"; +import { Serializable } from "@fluidframework/datastore-definitions"; +import { useMessageRelay } from "../../MessageRelayContext"; +import { TreeHeader } from "./TreeHeader"; + +/** + * Input to {@link EditableValueView} + */ +export interface EditableValueViewProps { + node: FluidObjectValueNode; + containerKey: string; + label: string; +} + +/** + * Constructs a editable text field to allow editing of a DDS' content. + */ +export function EditableValueView(props: EditableValueViewProps): React.ReactElement { + const { node, containerKey, label } = props; + const messageRelay = useMessageRelay(); + + /** + * value is the current value of the text field. + */ + const [value, setValue] = React.useState(String(node.value)); + + /** + * isEditing is whether or not the user is currently attempting to edit a value. If they are it will not update so their changes are not overwritten + */ + const [isEditing, setIsEditing] = React.useState(false); + + /** + * isType declares what type the edit will be + */ + const [editType, setEditType] = React.useState(typeof node.value); + + const textAreaRef = React.useRef(null); + + /** + * Keep the value in the input area up to date with the value of the node value as long as the user is not currently editing + */ + React.useEffect(() => { + if (isEditing === false) { + setValue(String(node.value)); + setEditType(typeof node.value); + } + }, [node.value, isEditing]); + + /** + * When starting to edit will select all of the text currently in the box + */ + const onFocus = React.useCallback(() => { + if (textAreaRef.current !== null) { + textAreaRef.current?.select(); + } + + setIsEditing(true); + }, []); + + /** + * Updates the value state with the user input while editing + */ + const onChange = React.useCallback( + (_ev: React.ChangeEvent, data: InputOnChangeData) => { + setValue(data.value); + }, + [], + ); + + /** + * When the field is "blur-ed" it reverts any changes to the field back to the + * value of the node and sets state to not editing + */ + const onBlur = React.useCallback(() => { + setValue(String(node.value)); + setIsEditing(false); + }, [node.value]); + + /** + * When changes are "commited" it will parse the data for the desired type {@link editType}. + * Then it will post an edit message + */ + const commitChanges = React.useCallback(() => { + let data: Serializable = value; + switch (editType) { + case "number": + data = Number(value); + break; + case "boolean": + data = value === "true" ? true : false; + break; + default: + data = String(value); + break; + } + const edit = { + fluidObjectId: node.fluidObjectId, + data, + }; + messageRelay.postMessage( + DataEdit.createMessage({ + containerKey, + edit, + }), + ); + }, [containerKey, editType, messageRelay, node.fluidObjectId, value]); + + /** + * Listens on keydown to be able to both escape and commit + */ + const onKeyDown = React.useCallback( + (event: React.KeyboardEvent) => { + const key = event.key; + + if (key === "Enter") { + commitChanges(); + event.currentTarget.blur(); + } + + if (key === "Escape") { + event.currentTarget.blur(); + } + }, + [commitChanges], + ); + + /** + * Converts editType to the Input form type. + * + * @remarks Will return `undefined` if the input edit type is not one the system knows about. + */ + function editTypeToInputType(): "number" | "text" | undefined { + switch (editType) { + case "number": + return "number"; + case "string": + return "text"; + default: + console.warn(`Unrecognized editType value "${editType}".`); + return undefined; + } + } + + const inputType = editTypeToInputType(); + if (inputType === undefined) { + // If the edit type is not one we recognize, do not allow (unsafe) editing. + return ( + + ); + } + + return ( + <> + + event.preventDefault()} + value={String(value)} + onChange={onChange} + onFocus={onFocus} + onBlur={onBlur} + onKeyDown={onKeyDown} + type={inputType} + /> + + ); +} diff --git a/packages/tools/devtools/devtools-view/src/components/data-visualization/FluidHandleView.tsx b/packages/tools/devtools/devtools-view/src/components/data-visualization/FluidHandleView.tsx index 678ce2bd96d1..67b3820e2539 100644 --- a/packages/tools/devtools/devtools-view/src/components/data-visualization/FluidHandleView.tsx +++ b/packages/tools/devtools/devtools-view/src/components/data-visualization/FluidHandleView.tsx @@ -85,7 +85,13 @@ export function FluidHandleView(props: FluidHandleViewProps): React.ReactElement if (visualTree === undefined) { const header = } />; return ; - } + } else { + const header = ; - return ; + return ( + + + + ); + } } diff --git a/packages/tools/devtools/devtools-view/src/components/data-visualization/FluidValueView.tsx b/packages/tools/devtools/devtools-view/src/components/data-visualization/FluidValueView.tsx index 973c5e6ed069..d35751d30c57 100644 --- a/packages/tools/devtools/devtools-view/src/components/data-visualization/FluidValueView.tsx +++ b/packages/tools/devtools/devtools-view/src/components/data-visualization/FluidValueView.tsx @@ -4,31 +4,47 @@ */ import React from "react"; -import { FluidObjectValueNode } from "@fluid-experimental/devtools-core"; +import { + ContainerDevtoolsFeature, + FluidObjectValueNode, + HasContainerKey, +} from "@fluid-experimental/devtools-core"; +import { useContainerFeaturesContext } from "../../ContainerFeatureFlagHelper"; +import { EditableValueView } from "./EditableValueView"; import { DataVisualizationTreeProps } from "./CommonInterfaces"; -import { TreeHeader } from "./TreeHeader"; import { TreeItem } from "./TreeItem"; +import { TreeHeader } from "./TreeHeader"; /** * {@link ValueView} input props. */ -export type FluidValueViewProps = DataVisualizationTreeProps; +export type FluidValueViewProps = DataVisualizationTreeProps & + HasContainerKey; /** * Render data with type VisualNodeKind.FluidValueNode and render its children. + * + * @remarks {@link ContainerFeaturesContext} must be set in order to use this component. */ export function FluidValueView(props: FluidValueViewProps): React.ReactElement { - const { label, node } = props; - - const metadata = JSON.stringify(node.metadata); + const { label, node, containerKey } = props; + const { containerFeatureFlags } = useContainerFeaturesContext(); + const editingEnabled = + containerFeatureFlags[ContainerDevtoolsFeature.ContainerDataEditing] === true && + node.editProps !== undefined; const header = ( - + <> + {editingEnabled === true ? ( + + ) : ( + + )} + ); return ; diff --git a/packages/tools/devtools/devtools-view/src/components/data-visualization/TreeDataView.tsx b/packages/tools/devtools/devtools-view/src/components/data-visualization/TreeDataView.tsx index c8522acc3cb1..c3f2e31905a6 100644 --- a/packages/tools/devtools/devtools-view/src/components/data-visualization/TreeDataView.tsx +++ b/packages/tools/devtools/devtools-view/src/components/data-visualization/TreeDataView.tsx @@ -46,7 +46,7 @@ export function TreeDataView(props: TreeDataViewProps): React.ReactElement { * FluidObjectNode with primitive value. */ case VisualNodeKind.FluidValueNode: - return ; + return ; /** * Unknown data type. */ diff --git a/packages/tools/devtools/devtools-view/src/components/data-visualization/TreeItem.tsx b/packages/tools/devtools/devtools-view/src/components/data-visualization/TreeItem.tsx index ab9625d9e53a..b6742a1e44e7 100644 --- a/packages/tools/devtools/devtools-view/src/components/data-visualization/TreeItem.tsx +++ b/packages/tools/devtools/devtools-view/src/components/data-visualization/TreeItem.tsx @@ -35,6 +35,7 @@ export function TreeItem(props: TreeItemProps): React.ReactElement { return ( {header} + {children} ); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e44ca6857e74..59048dc151b1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -11016,6 +11016,7 @@ importers: '@fluidframework/container-utils': workspace:~ '@fluidframework/core-interfaces': workspace:~ '@fluidframework/counter': workspace:~ + '@fluidframework/datastore-definitions': workspace:~ '@fluidframework/driver-definitions': workspace:~ '@fluidframework/eslint-config-fluid': ^2.0.0 '@fluidframework/map': workspace:~ @@ -11056,6 +11057,7 @@ importers: '@fluidframework/container-utils': link:../../../loader/container-utils '@fluidframework/core-interfaces': link:../../../common/core-interfaces '@fluidframework/counter': link:../../../dds/counter + '@fluidframework/datastore-definitions': link:../../../runtime/datastore-definitions '@fluidframework/map': link:../../../dds/map '@fluidframework/matrix': link:../../../dds/matrix '@fluidframework/protocol-definitions': 1.2.0 @@ -11273,6 +11275,7 @@ importers: '@fluidframework/common-utils': 1.1.1 '@fluidframework/container-definitions': link:../../../common/container-definitions '@fluidframework/container-loader': link:../../../loader/container-loader + '@fluidframework/datastore-definitions': link:../../../runtime/datastore-definitions '@fluidframework/driver-definitions': link:../../../common/driver-definitions '@fluidframework/protocol-definitions': 1.2.0 '@fluidframework/telemetry-utils': link:../../../utils/telemetry-utils @@ -11285,7 +11288,6 @@ importers: '@fluidframework/build-common': 1.2.0 '@fluidframework/build-tools': 0.21.0 '@fluidframework/core-interfaces': link:../../../common/core-interfaces - '@fluidframework/datastore-definitions': link:../../../runtime/datastore-definitions '@fluidframework/eslint-config-fluid': 2.0.0_kufnqfq7tb5rpdawkdb6g5smma '@fluidframework/shared-object-base': link:../../../dds/shared-object-base '@fluidframework/test-runtime-utils': link:../../../runtime/test-runtime-utils From 08bff4663a9f96104d4e3ba878507cc22afdcaf3 Mon Sep 17 00:00:00 2001 From: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Date: Tue, 1 Aug 2023 11:26:32 -0700 Subject: [PATCH 7/8] test(tree): add sequence changeset persisted format coverage (#16636) --- .../files/nested-sequence-change.json | 37 ++++++++++++++++ .../dds/tree2/src/test/snapshots/testTrees.ts | 42 ++++++++++++++++++- 2 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 experimental/dds/tree2/src/test/snapshots/files/nested-sequence-change.json diff --git a/experimental/dds/tree2/src/test/snapshots/files/nested-sequence-change.json b/experimental/dds/tree2/src/test/snapshots/files/nested-sequence-change.json new file mode 100644 index 000000000000..a6be20b260f0 --- /dev/null +++ b/experimental/dds/tree2/src/test/snapshots/files/nested-sequence-change.json @@ -0,0 +1,37 @@ +{ + "type": 1, + "tree": { + "indexes": { + "type": 1, + "tree": { + "EditManager": { + "type": 1, + "tree": { + "String": { + "type": 2, + "content": "{\"trunk\":[{\"change\":{\"maxId\":1,\"changes\":[{\"fieldKey\":\"rootFieldKey\",\"fieldKind\":\"Sequence\",\"change\":[{\"type\":\"Insert\",\"content\":[{\"type\":\"SeqMap\"}],\"id\":0,\"changes\":{\"fieldChanges\":[{\"fieldKey\":\"foo\",\"fieldKind\":\"Sequence\",\"change\":[{\"type\":\"Insert\",\"content\":[{\"type\":\"SeqMap\"}],\"id\":1}]}]}}]}]},\"revision\":\"beefbeef-beef-4000-8000-000000000004\",\"sequenceNumber\":-9007199254740990,\"sessionId\":\"beefbeef-beef-4000-8000-000000000001\"}],\"branches\":[]}" + } + } + }, + "Schema": { + "type": 1, + "tree": { + "SchemaString": { + "type": 2, + "content": "{\"version\":\"1.0.0\",\"treeSchema\":[{\"name\":\"SeqMap\",\"mapFields\":{\"kind\":\"Sequence\",\"types\":[\"SeqMap\"]},\"structFields\":[],\"value\":0}],\"rootFieldSchema\":{\"kind\":\"Sequence\",\"types\":[\"SeqMap\"]}}" + } + } + }, + "Forest": { + "type": 1, + "tree": { + "ForestTree": { + "type": 2, + "content": "\"[{\\\"type\\\":\\\"SeqMap\\\",\\\"fields\\\":{\\\"foo\\\":[{\\\"type\\\":\\\"SeqMap\\\"}]}}]\"" + } + } + } + } + } + } +} \ No newline at end of file diff --git a/experimental/dds/tree2/src/test/snapshots/testTrees.ts b/experimental/dds/tree2/src/test/snapshots/testTrees.ts index 388081ad5985..2f4bf1d8d181 100644 --- a/experimental/dds/tree2/src/test/snapshots/testTrees.ts +++ b/experimental/dds/tree2/src/test/snapshots/testTrees.ts @@ -7,7 +7,7 @@ import { SharedTreeTestFactory, TestTreeProviderLite, initializeTestTree } from import { brand, useDeterministicStableId } from "../../util"; import { FieldKey, UpPath, ValueSchema, rootFieldKey } from "../../core"; import { ISharedTree, ISharedTreeView } from "../../shared-tree"; -import { SchemaBuilder, singleTextCursor } from "../../feature-libraries"; +import { FieldKinds, SchemaBuilder, singleTextCursor } from "../../feature-libraries"; const fieldKeyA: FieldKey = brand("FieldA"); const fieldKeyB: FieldKey = brand("FieldB"); @@ -92,6 +92,46 @@ export function generateTestTrees(): { name: string; tree: ISharedTree }[] { ); }; + const provider = new TestTreeProviderLite(1, new SharedTreeTestFactory(onCreate)); + return provider.trees[0]; + }, + }, + { + name: "nested-sequence-change", + tree: () => { + const builder = new SchemaBuilder("has-sequence-map"); + const seqMapSchema = builder.mapRecursive( + "SeqMap", + SchemaBuilder.fieldRecursive(FieldKinds.sequence, () => seqMapSchema), + ); + const docSchema = builder.intoDocumentSchema( + SchemaBuilder.fieldSequence(seqMapSchema), + ); + const onCreate = (tree: ISharedTree) => { + tree.storedSchema.update(docSchema); + tree.transaction.start(); + // We must make this shallow change to the sequence field as part of the same transaction as the + // nested change. Otherwise, the nested change will be represented using the generic field kind. + tree.editor + .sequenceField({ + parent: undefined, + field: rootFieldKey, + }) + .insert(0, [singleTextCursor({ type: brand("SeqMap") })]); + // The nested change + tree.editor + .sequenceField({ + parent: { + parent: undefined, + parentField: rootFieldKey, + parentIndex: 0, + }, + field: brand("foo"), + }) + .insert(0, [singleTextCursor({ type: brand("SeqMap") })]); + tree.transaction.commit(); + }; + const provider = new TestTreeProviderLite(1, new SharedTreeTestFactory(onCreate)); return provider.trees[0]; }, From 1b21a44bc956fcd60ddd1828b4882956feba396e Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Tue, 1 Aug 2023 11:46:51 -0700 Subject: [PATCH 8/8] ci(sync-branch): Check out main branch by default (#16618) --- .github/workflows/sync-branch.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/sync-branch.yml b/.github/workflows/sync-branch.yml index a4d67b48bc6e..5c12e719b50e 100644 --- a/.github/workflows/sync-branch.yml +++ b/.github/workflows/sync-branch.yml @@ -65,6 +65,9 @@ jobs: with: fetch-depth: "0" # all history persist-credentials: false + # always check out the main branch by default. Otherwise the starting branch is based on the + # triggering event which can seem unpredictable when the workflow is triggered in different branches. + ref: main - uses: pnpm/action-setup@c3b53f6a16e57305370b4ae5a540c2077a1d50dd # ratchet:pnpm/action-setup@v2 - uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # ratchet:actions/setup-node@v3 with: