build(common-utils): modernize build, lint, and test setup#27319
build(common-utils): modernize build, lint, and test setup#27319ChumpChief wants to merge 19 commits into
Conversation
Prep for switching the package to ESM (Node16) emit. Adding the explicit ".js" suffixes is a no-op under the current TypeScript compiler settings, so it can land safely ahead of the tsconfig swap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adopt the same tsconfig topology used by client-utils and protocol-definitions: - tsconfig.json now extends build-common's tsconfig.node16.json and emits ESM into ./lib (was the role of tsconfig.esnext.json). - New tsconfig.cjs.json extends tsconfig.json and emits CJS into ./dist via fluid-tsc commonjs. - tsconfig.esnext.json removed. - Test tsconfigs migrated to extend tsconfig.test.node16.json. The mocha config now has both an ESM (tsconfig.json) and a CJS (tsconfig.cjs.json) variant; the jest config is renamed tsconfig.cjs.json since it must remain CJS (rewire). package.json: - "type": "module" with an exports map that resolves node vs. browser entry points for both import and require, replacing the older main/module/browser fields. - tsc script now runs fluid-tsc commonjs and copies the cjs package.json shim to ./dist. - build:esnext targets the new tsconfig.json. - build:test:mocha now drives both the ESM and CJS variants. - fluidBuild task graph updated for the split mocha builds and the separate build:esnext step. Source updates required by Node16 module resolution: - Drop internal-module imports (sha.js/sha1, sha.js/sha256, lodash/cloneDeep) in favor of top-level imports. - Add the missing .js suffix on the dynamic ./hashFileNode import. Strictness: - exactOptionalPropertyTypes and noUncheckedIndexedAccess are disabled in tsconfig.json. The package is deprecated, so we opt out of the stricter base-config defaults rather than churn the source. api-extractor: - api-extractor.json now extends api-extractor-base.esm.no-legacy (instead of cjs). - api-extractor-lint.json points at lib/index.d.ts. eslint.config.mts switches from projectService to an explicit project list so the renamed jest tsconfig is picked up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cosmetic only \u2014 reorder the mocha tsconfig types array to ["mocha", "node"] to match client-utils. attw was attempted as a follow-up but reverted: adding the @arethetypeswrong/cli devDependency forces a lockfile refresh, which trips the workspace's no-downgrade trust policy on a pre-existing rewire@5.0.0 \u2192 eslint@6.8.0 \u2192 cross-spawn@6.0.6 \u2192 semver@5.7.2 chain. Bumping rewire is out of scope for this change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The package now declares "type": "module", which makes Node treat .js files as ESM. Both jest config files use CommonJS module.exports syntax, so they fail to load with: ReferenceError: module is not defined in ES module scope Rename them to .cjs so Node treats them as CommonJS, matching the client-utils convention. No content changes \u2014 jest auto-discovers both .cjs files by name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the legacy npm-script orchestration of build, build:compile, build:commonjs, lint, and lint:fix with the fluid-build task graph form used by client-utils and protocol-definitions: build -> fluid-build . --task build build:commonjs-> fluid-build . --task commonjs build:compile -> fluid-build . --task compile lint -> fluid-build . --task lint lint:fix -> fluid-build . --task lint:fix The existing fluidBuild.tasks block in package.json already declares the dependency graph, so fluid-build picks it up unchanged. Verified `npm run build` runs all 12 tasks, plus mocha (53), jest (19), and lint stay green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Bump @fluid-tools/build-cli and @fluidframework/build-tools from
^0.63.0 to ^0.65.0 (matches the workspace-wide bump in
7f856ce557cef04023a6dcfbb7eebbeedd30b95d).
- Add the trust-policy exclusions that bump pulls in transitively
(@octokit/endpoint@9.0.6, semver@5.7.2 || 6.3.1).
- Regenerate src/test/types/validateCommonUtilsPrevious.generated.ts
with the new build-cli; the only changes are a header tweak and a
switch from a relative import to a package self-import:
-import type * as current from "../../index.js";
+import type * as current from "@fluidframework/common-utils";
That self-import surfaced a real bug in our modernized exports map.
The map points each platform entry directly at the platform file:
node import -> lib/indexNode.js
default import -> lib/indexBrowser.js
but those two files only carried the platform-specific Buffer/hash/
performance bindings. The bulk of the API (assert, Heap,
NumberComparer, toUtf8, ...) lived in index.ts and was unreachable
through the package self-import.
Fix by adopting the client-utils convention:
- index.ts is now a stub that just re-exports indexNode (kept only
so the type-test generator can resolve a canonical entry).
- indexNode.ts and indexBrowser.ts each list the full public API
for their platform, with bufferNode/hashFileNode swapped for
bufferBrowser/hashFileBrowser.
Side effect: api-report now records "(No @packageDocumentation
comment for this package)" because the comment moved off the
canonical entry; api-extractor only inspects that one. This matches
the existing client-utils api-report layout.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bump six devDependencies to the versions client-utils ships with: @types/base64-js ^1.3.0 -> ^1.3.2 cross-env ^7.0.3 -> ^10.1.0 jest-junit ^10.0.0 -> ^16.0.0 mocha ^10.8.2 -> ^11.7.5 rewire ^5.0.0 -> ^9.0.1 ts-node ^10.9.1 -> ^10.9.2 Side effect: the rewire 5 -> 9 bump drops the legacy eslint@6 -> cross-spawn@6 -> semver@5.7.2 chain that previously forced a trust-policy exclusion on semver@5.7.2 just to install. The semver exclusion is retained because semver@6.3.1 is still pulled in elsewhere. Add chokidar@4.0.3 to the workspace trust-policy exclusions; mocha@11 pulls it in transitively (precedent already in the root workspace). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
base64-js@1.5.x ships its own .d.ts files, so the @types/base64-js package is now a no-op stub. pnpm flagged it as deprecated: WARN deprecated @types/base64-js@1.5.0: This is a stub types definition. base64-js provides its own type definitions, so you do not need this installed. Build, mocha (53), and jest (19) stay green without it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The earlier attempt to add @arethetypeswrong/cli was blocked by a trust-downgrade error on semver@5.7.2, pulled in transitively through rewire@5 -> eslint@6 -> cross-spawn@6. The subsequent rewire@5 -> ^9 bump removed that chain, so the install now goes through cleanly. Adds the script and the devDependency at ^0.15.2 (matches protocol-definitions; client-utils uses ^0.18.2 but staying on avoids pulling in a newer toolchain). attw passes all four resolution modes (node10, node16 CJS/ESM, bundler) — confirms the modernized exports map is correct. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Match the protocol-definitions pattern of running attw as part of lint: lint: fluid-build . --task lint --task check:are-the-types-wrong Verified `npm run lint` runs the new attw task alongside the existing 9 lint tasks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop two pieces of legacy plumbing that no other modernized package
in the repo carries:
- --typescript-compiler-folder ./node_modules/typescript:
forces api-extractor to use the package's own typescript install
instead of its bundled one. api-extractor 7.58.1 ships with
typescript 5.9.3 already, and the analyzer doesn't care which
compiler emitted the .d.ts files it inspects.
- copyfiles -u 1 "./_api-extractor-temp/doc-models/*" ../../../_api-extractor-temp/:
mirrors the generated api.json into a repo-root temp directory.
No tooling reads from there anymore; downstream docs aggregation
walks each package's own ./_api-extractor-temp/doc-models/.
After this change build:docs / ci:build:docs match the
protocol-definitions form:
build:docs -> api-extractor run --local
ci:build:docs -> api-extractor run
Verified `npm run build`, `npm run ci:build`, `npm run test:mocha`,
`npm run test:jest`, `npm run lint` (with attw chain) all pass.
The copyfiles devDependency is retained — the `tsc` script still
uses it to drop the cjs package.json shim into ./dist.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wire mocha through @fluid-internal/mocha-test-setup so the modernized ESM test build under lib/test/mocha is actually executed (it was being compiled but never run). Add a minimal .mocharc.cjs that delegates to getFluidTestMochaConfig, mirroring the standalone-workspace pattern used by tools/api-markdown-documenter and tools/benchmark, and replace the bespoke :report script chain with the client-utils-style test:mocha (esm + cjs), test:coverage, and ci:test scripts. ESM execution exposed two latent bugs that worked under the old CJS-only mocha runner: * rangeTracker.ts pulled cloneDeep via a named import from lodash@4, which has no named ESM exports. Switch to the single-function lodash.clonedeep package (with @types/lodash.clonedeep) so the default import resolves cleanly under Node ESM. * indexNode.ts re-exported Buffer as a value, but Buffer is declared with 'declare class' in bufferNode.ts and has no runtime presence. Re-export it as 'type Buffer' to match the client-utils convention and stop Node's ESM loader from rejecting the missing named binding. While here, restore prettier formatting on the indexNode/indexBrowser re-export lists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the standalone hand-rolled flat config with the published @fluidframework/eslint-config-fluid v10 'recommended' config, mirroring the pattern client-utils uses internally (which consumes the same package via the workspace catalog). This resolves the AB#59243 TODO that motivated the standalone config in the first place. Side benefits: * drops five direct devDeps that the shared config now provides transitively: @eslint/js, @rushstack/eslint-plugin, eslint-plugin-import, eslint-plugin-unicorn, typescript-eslint * picks up the import-x / depend / eslint-comments / promise / jsdoc rule sets that the shared config layers on Common-utils is in maintenance for deprecation, so rather than fixing the new violations the shared config surfaces, disable the rules package-wide. The disabled set covers existing source patterns (deprecated re-exports, node 'events' import, raw any, lodash.clonedeep ban, charCodeAt vs codePointAt, 'utf-8' string, etc.) plus a couple of warnings tied to legacy inline disable comments (reportUnusedDisableDirectives, eslint-comments/require-description). The non-standard test directory layout still requires overriding projectService and providing an explicit project list, matching what client-utils does. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Aligns with the version client-utils pins. attw still reports a clean result across all four resolution modes (node10, node16 CJS, node16 ESM, bundler). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The bump-version script (npm version minor with no-push/no-git-tag-version) predates the modern release-group machinery and has no remaining callers in the repo (no pipeline yaml, no build-tools script, no other package script). Reference packages such as client-utils and protocol-definitions do not carry it. Versioning for this package now flows through the same flub bump / flub release tooling as the rest of the client release group. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (4558 lines, 43 files), I've queued these reviewers:
How this works
|
🔭 PR Review Fleet ReportNote This report is generated by an experimental AI review fleet and is provided as a beta feature. Findings are a starting point for discussion, not a gate. Use your own judgement. Verdict: 0 Spicy, 0 Pungent, 1 Smelly Findings
|
PR feedback flagged that lodash.clonedeep@4.5.0 is unmaintained (last
published in 2017) and never received the security patches that lodash
proper got. RangeTracker.ranges is loaded from snapshot data, so a
crafted document could plausibly carry a __proto__ payload through
cloneDeep into Object.prototype.
structuredClone (Node >=17, modern browsers) is built in, dependency-
free, and safe for the plain {primary, secondary, length} objects that
IRange[] contains. Removing lodash.clonedeep also gets rid of one more
unmaintained third-party dep that would otherwise turn into a
recurring CG alert magnet on this deprecated package -- which is
exactly the maintenance cost this PR is trying to drive down.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Satisfies the npm-package-exports-apis-linted policy by adding per-entrypoint api-extractor lint configs and check:exports scripts that match client-utils' pattern. The previous check:release-tags script pointed at ./lib/index.d.ts, which isn't in the exports map, so it provided no real coverage; replaced by the new per-entrypoint flow. Tagged the seven Browser-side exports (Uint8ArrayToString, stringToBuffer, bufferToString, isArrayBuffer, IsoBuffer, hashFile, gitHashFile) as @internal to mirror their Node counterparts and clear ae-missing-release-tag. No api-report diff: the package's api-extractor.json extends no-legacy, so @internal symbols are already excluded from the generated reports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
There was a problem hiding this comment.
Pull request overview
This PR modernizes the deprecated @fluidframework/common-utils workspace to align with the repo’s current build/lint/test and ESM/exports patterns (similar to client-utils), so future maintenance (tooling updates, CG responses, pipeline alignment) is cheaper and more uniform.
Changes:
- Migrates to Node16 ESM-first TypeScript config and updates source/test imports to explicit
.jsextensions. - Reworks package entrypoints/exports to support proper self-import usage across Node/browser and ESM/CJS, plus adds export-validation and
arethetypeswrongchecks. - Updates build, lint, and test wiring (fluid-build tasks, eslint-config-fluid flat config, mocha/jest configs, refreshed devDependencies).
Reviewed changes
Copilot reviewed 40 out of 43 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| common/lib/common-utils/tsconfig.json | Switches to Node16 ESM tsconfig base and adjusts output to lib/, relaxing a couple of strictness flags for deprecated code. |
| common/lib/common-utils/tsconfig.esnext.json | Removes the old ESNext-specific tsconfig. |
| common/lib/common-utils/tsconfig.cjs.json | Adds a dedicated CJS build tsconfig (for fluid-tsc commonjs) outputting to dist/. |
| common/lib/common-utils/src/trace.ts | Updates import to .js extension (now ESM-style). |
| common/lib/common-utils/src/timer.ts | Updates internal imports to .js extensions. |
| common/lib/common-utils/src/test/types/validateCommonUtilsPrevious.generated.ts | Updates generated type-test import to package self-import and refreshes generator comment. |
| common/lib/common-utils/src/test/types/tsconfig.json | Moves type-test build output to lib/ and aligns test tsconfig with Node16 test base. |
| common/lib/common-utils/src/test/mocha/typedEventEmitter.spec.ts | Switches test imports to explicit entrypoint/module paths with .js. |
| common/lib/common-utils/src/test/mocha/tsconfig.json | Aligns mocha test TS config with Node16 test base and outputs to lib/. |
| common/lib/common-utils/src/test/mocha/tsconfig.cjs.json | Adds a CJS-specific mocha test build config emitting into dist/. |
| common/lib/common-utils/src/test/mocha/timer.spec.ts | Switches test import to explicit index.js entrypoint with .js. |
| common/lib/common-utils/src/test/mocha/rangeTracker.spec.ts | Switches test import to explicit index.js entrypoint with .js. |
| common/lib/common-utils/src/test/mocha/promiseCache.spec.ts | Switches test import to explicit index.js entrypoint with .js. |
| common/lib/common-utils/src/test/mocha/idleTaskScheduler.spec.ts | Switches test import to explicit module path with .js. |
| common/lib/common-utils/src/test/mocha/assert.spec.ts | Switches test import to explicit module path with .js. |
| common/lib/common-utils/src/test/jest/tsconfig.cjs.json | Aligns jest test compilation config with Node16 test base and references the package CJS build config. |
| common/lib/common-utils/src/test/jest/gitHash.spec.ts | Switches test import to explicit module path with .js. |
| common/lib/common-utils/src/test/jest/buffer.spec.ts | Switches test imports to explicit module paths with .js. |
| common/lib/common-utils/src/rangeTracker.ts | Replaces lodash cloneDeep usage with structuredClone. |
| common/lib/common-utils/src/indexNode.ts | Builds a complete Node entrypoint surface via explicit re-exports and adds @packageDocumentation. |
| common/lib/common-utils/src/indexBrowser.ts | Builds a complete browser entrypoint surface via explicit re-exports and adds @packageDocumentation. |
| common/lib/common-utils/src/index.ts | Converts index.ts into a stub re-export for tooling compatibility. |
| common/lib/common-utils/src/hashFileNode.ts | Updates sha.js import style and .js extension on internal type import. |
| common/lib/common-utils/src/hashFileBrowser.ts | Updates imports/paths for ESM .js extensions; adds @internal tags to deprecated APIs. |
| common/lib/common-utils/src/bufferBrowser.ts | Adds @internal tags to deprecated browser buffer utilities. |
| common/lib/common-utils/src/base64Encoding.ts | Updates import path to ESM .js extension. |
| common/lib/common-utils/pnpm-workspace.yaml | Adds trust-policy exclusions with rationale for specific transitive versions. |
| common/lib/common-utils/package.json | Converts package to ESM (type: module), adds conditional exports (node/default + import/require + types), updates scripts, and refreshes deps/devDeps. |
| common/lib/common-utils/jest.config.cjs | Adds a CJS jest config (puppeteer preset + junit reporter). |
| common/lib/common-utils/jest-puppeteer.config.cjs | Adds a CJS jest-puppeteer launch config for CI environments. |
| common/lib/common-utils/eslint.config.mts | Replaces bespoke flat config with @fluidframework/eslint-config-fluid and customizes TS parser project list + disables for existing deprecated violations. |
| common/lib/common-utils/api-report/common-utils.public.api.md | Updates generated API report content (now indicates missing package documentation for the configured entrypoint). |
| common/lib/common-utils/api-report/common-utils.beta.api.md | Updates generated API report content (now indicates missing package documentation for the configured entrypoint). |
| common/lib/common-utils/api-report/common-utils.alpha.api.md | Updates generated API report content (now indicates missing package documentation for the configured entrypoint). |
| common/lib/common-utils/api-extractor/api-extractor-lint-indexNode.esm.json | Adds API-extractor lint config for the Node ESM entrypoint. |
| common/lib/common-utils/api-extractor/api-extractor-lint-indexNode.cjs.json | Adds API-extractor lint config for the Node CJS entrypoint. |
| common/lib/common-utils/api-extractor/api-extractor-lint-indexBrowser.esm.json | Adds API-extractor lint config for the browser ESM entrypoint. |
| common/lib/common-utils/api-extractor/api-extractor-lint-indexBrowser.cjs.json | Adds API-extractor lint config for the browser CJS entrypoint. |
| common/lib/common-utils/api-extractor/api-extractor-lint-bundle.json | Adds API-extractor lint config for the “bundle” entrypoint. |
| common/lib/common-utils/api-extractor.json | Switches API Extractor base config to the ESM (no-legacy) base. |
| common/lib/common-utils/api-extractor-lint.json | Removes the old single-entrypoint API-extractor lint config. |
| common/lib/common-utils/.mocharc.cjs | Adds mocha config via @fluid-internal/mocha-test-setup. |
base64Encoding.ts and trace.ts both imported through ./indexNode.js, which worked under the legacy 'browser' field that remapped indexNode to indexBrowser at bundle time. With this PR's switch to conditional exports, that browser field is gone and those relative imports always resolve to the Node entrypoint at runtime, dragging Node-only modules (sha.js, the Node Buffer surface, etc.) into browser bundles. * Split base64Encoding.ts into base64EncodingBrowser.ts and base64EncodingNode.ts, each importing IsoBuffer from the platform-correct buffer module. Mirrors the client-utils pattern. indexBrowser/indexNode re-export from the matching half. * trace.ts now imports performance directly from ./performanceIsomorphic.js, which is already platform-isomorphic and avoids the entrypoint round-trip entirely. No public API change (no api-report diff). All tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a @packageDocumentation comment to the three index files (index.ts, indexBrowser.ts, indexNode.ts) and notes that the package is deprecated in favor of @fluid-internal/client-utils. Clears the '(No @packageDocumentation comment for this package)' line from the generated api-reports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Tracking: AB#73445.
@fluidframework/common-utilsis deprecated but still lives in this repo as its own pnpm workspace. Leaving it on its old build/lint/test setup makes it expensive to respond to Component Governance alerts and to apply uniform changes to build tooling and pipelines — each one has to be handled twice (once for the modern packages, once for this one's bespoke world). Bringing it in line withclient-utilsandprotocol-definitionsmakes future maintenance roughly free.Updates
.js-extension relative imports, and.cjsjest configs (matchesclient-utils).Heap,assert, etc. were unreachable through the modern exports map).@fluid-tools/build-cliand@fluidframework/build-toolsto^0.65.0; refreshed devDependencies (mocha 11,rewire 9,cross-env 10,jest-junit 16,ts-node 10.9); added@arethetypeswrong/cli ^0.18.2and chainedcheck:are-the-types-wrongintolint.@fluid-internal/mocha-test-setupvia a one-line.mocharc.cjs, dropping the bespoke:reportscript chain. ESM-built mocha tests now actually run (were being compiled but never executed).@fluidframework/eslint-config-fluid@^10recommended(resolves the AB#59243 TODO). Existing violations are silenced with package-wide disables — this package is in deprecation mode and is not getting source-level cleanups.lodash→lodash.clonedeep(the named import was broken under ESM); dropped the deprecated@types/base64-jsstub; removed the vestigialbump-versionscript and stalebuild:docsflags.Reviewer Guidance
The review process is outlined on this wiki page.
Each commit is scoped to one logical change for easier bisecting. Verified locally:
build,ci:build,test:mocha(53 ESM + 53 CJS),test:jest(19),lint(incl. attw across all four resolution modes).