Skip to content

build(tree): enable importHelpers to dedupe TypeScript runtime helpers#27531

Open
TommyBrosman wants to merge 3 commits into
microsoft:mainfrom
TommyBrosman:tbrosman/enable-import-helpers
Open

build(tree): enable importHelpers to dedupe TypeScript runtime helpers#27531
TommyBrosman wants to merge 3 commits into
microsoft:mainfrom
TommyBrosman:tbrosman/enable-import-helpers

Conversation

@TommyBrosman

Copy link
Copy Markdown
Contributor

Description

Enable importHelpers (with a tslib dependency) in @fluidframework/tree. With this on, the TypeScript compiler emits import { ... } from "tslib" instead of inlining the runtime helpers (e.g. __assign, __awaiter, __spreadArray) into every module that uses them. Webpack can then dedupe those helpers down to a single shared copy, shrinking the bundle.

Measured impact (parsed bytes), comparing against the parent revision:

Entrypoint Base Current Diff %
sharedTree.js 401,086 392,105 −8,981 -2.2%
fluidFrameworkAll.js 1,326,045 1,317,064 -8,981 -0.7%

Gzipped: sharedTree.js -1,948 B, fluidFrameworkAll.js -1,949 B.

The mechanism is visible in the per-package breakdown: inlined helpers drop out of @fluidframework/fluid-framework (-12,499 B) while a single shared tslib module is added (+3,902 B), for a net ~-8,981 B per affected entrypoint. All other entrypoints are unchanged.

Changes:

  • Add "importHelpers": true to packages/dds/tree/tsconfig.json.
  • Add tslib as a dependency in packages/dds/tree/package.json (and corresponding lockfile update).

Reviewer Guidance

  • tslib is added as a runtime dependency. This is not a widely-used dependency in our repo (client-logger/fluid-telemetry uses it, so does the devtools browser extension). Are there any potential issues if tree depends on it?
  • The bundle numbers above were produced with flub bundle collect-and-compare against the commit's parent revision. See feat(build-cli): bundle size collection and comparison tooling #27151 for the version of the tools used.

Copilot AI review requested due to automatic review settings June 10, 2026 20:12
@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (171 lines, 7 files), I've queued these reviewers:

  • Correctness — logic errors, race conditions, lifecycle issues
  • Security — vulnerabilities, secret exposure, injection
  • API Compatibility — breaking changes, release tags, type design
  • Performance — algorithmic regressions, memory leaks
  • Testing — coverage gaps, hollow tests

How this works

  • Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

  • Tick Start review below to dispatch the review fleet.

  • After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

  • This comment updates as new commits land; your reviewer selections are preserved.

  • Start review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

"compilerOptions": {
"rootDir": "./src",
"outDir": "./lib",
// Avoid emitting __classPrivateFieldGet/Set, __esDecorate, __runInitializers, etc.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For at least __classPrivateFieldGet, I think it would be much better to just target es version we said we required when we released FF 2.0, ES2022, so we use the build in JS private fields, instead of emitting legacy adapter stuff with size and time overhead. We have known for a long time doing so would give a significant bundle size and likely performance win, and also better devX. I have an old PR for that in #25398

If we decide not to go that route, or it doesn't address all the cases, I think whatever choice we make should apply to all of FF Client, and not just tree.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think setting a unified policy for client packages makes sense. Only a subset will see noticeable size reductions though:

  • dds/tree
  • runtime/container-runtime
  • loader/container-loader
  • dds/sequence
  • dds/shared-object-base
  • framework/aqueduct

The ES2022 change makes sense to me, and I hope we get it prioritized. I can update my comment to mention that this __classPrivateFieldGet will go away with the ES bump. I think there is value to landing the tree changes first: we get an idea as to whether using tslib works well in client packages.

Enabling importHelpers in @fluidframework/tree (prior commit) added a tslib@^2.8.1 dependency. syncpack's single-version-per-workspace rule then failed because four packages still pinned tslib@^1.10.0:

- examples/client-logger/app-insights-logger (devDep)
- packages/framework/client-logger/app-insights-logger (devDep)
- packages/framework/client-logger/fluid-telemetry (devDep)
- packages/tools/devtools/devtools-browser-extension (private/internal)

Why this does not affect functionality: none of these four packages set importHelpers in their tsconfig nor import tslib directly, so the dependency is vestigial and the resolved helper code is unchanged. tslib only ships emit helpers (e.g. __awaiter, __spreadArray) whose behavior is stable across the v1->v2 range, and three of the four entries are dev-only; devtools-browser-extension is a private package. The lockfile changes are strictly tslib-scoped (specifier ^1.10.0 -> ^2.8.1, resolved 1.14.1 -> 2.8.1). No consumer-facing API or shipped runtime behavior changes, so no changeset is required.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants