Skip to content

refactor: move manifest template resolver into @microsoft/app-manifest - #16270

Draft
iclanton wants to merge 2 commits into
OfficeDev:devfrom
iclanton:odsp/manifest-api-extraction
Draft

refactor: move manifest template resolver into @microsoft/app-manifest#16270
iclanton wants to merge 2 commits into
OfficeDev:devfrom
iclanton:odsp/manifest-api-extraction

Conversation

@iclanton

@iclanton iclanton commented Jul 2, 2026

Copy link
Copy Markdown

Summary

Moves the DriverContext-free parts of fx-core's manifest templating
($[file('...')] function expansion and ${{ENV}} variable expansion) down into
@microsoft/app-manifest, so the logic can be consumed directly from the manifest
package without depending on @microsoft/teamsfx-core, a DriverContext,
localization, or UserError.

This is a behavior-preserving refactor. fx-core now delegates to the moved logic
and keeps only its host concerns (telemetry + localized error mapping).

What moves where

Relocated into @microsoft/app-manifest (single source of truth):

  • expandEnvironmentVariable / getEnvironmentVariables — moved verbatim from
    fx-core/src/component/utils/common.ts.
  • The file() function resolver and file reader — moved from
    fx-core/src/component/utils/envFunctionUtils.ts, decoupled from DriverContext
    and raising plain typed errors that carry the offending path/token.
  • The ManifestType enum.
  • Also exposes expandFileFunctionMacros (the resolution loop, returning the
    expanded content plus a function count for host telemetry) and resolveManifest,
    the host-agnostic counterpart of fx-core's getResolvedManifest.
  • Reuses the package's existing strip-bom and fs-extra usage — no new deps.

Left in fx-core (host concerns only):

  • common.ts re-exports expandEnvironmentVariable / getEnvironmentVariables
    from @microsoft/teamsfx-api, so all existing call sites are unchanged.
  • expandVariableWithFunction delegates its loop to expandFileFunctionMacros,
    keeping the telemetry event and mapping the resolver's plain errors to localized
    UserError / fx-core FileNotFoundError via toFxError.
  • getResolvedManifest is unchanged and keeps composing these primitives (its
    telemetry is preserved exactly).

Testing

  • @microsoft/app-manifest unit suite: 184 passing (manifestTemplate.ts at
    100% statements / 95% branch / 100% functions).
  • @microsoft/teamsfx-core: the affected suites plus every consumer of the
    re-exported env functions (aad manifest builder, common utils, scenarios,
    declarative-agent generator) pass — no regressions.
  • Affected tests were updated to drive resolution through real temp files instead
    of fs mocks, since file reads now happen inside @microsoft/app-manifest.
  • Lint / lint-staged clean on all changed files.

Notes / follow-ups

  • Public API namingresolveManifest, expandFileFunctionMacros, and
    processManifestFunction are new public exports on @microsoft/app-manifest.
    Happy to adjust the names/shape if the team prefers a different surface.
  • Schema-validation packaging — separate from this PR, but noting it here:
    AppManifestUtils.fetchSchema is local-first (maps the manifest $schema URL to
    a bundled build/json-schemas/... file, with the network only as a fallback).
    The source bundles the schemas (src/json-schemas/, prebuild: copy-json-schema,
    files: ["build/**/*"]), but the published tarball does not include
    build/json-schemas/, so offline validation silently falls back to a network
    fetch. Fixing the publish so the schemas ship would make schema validation fully
    hermetic with no code change. Can file/fix separately if useful.
  • CI — I validated the affected suites and all consumers of the re-exported
    functions locally; a full @microsoft/teamsfx-core suite run in CI is worth a
    glance (the 3 unrelated pre-existing failures need bundled template artifacts that
    a plain bootstrap doesn't produce).

Opened as a draft to gather feedback on the API surface before finalizing.

Comment thread packages/manifest/src/manifestTemplate.ts Fixed
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.94656% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.08%. Comparing base (cb22bf8) to head (23a1773).
⚠️ Report is 18 commits behind head on dev.

Files with missing lines Patch % Lines
...es/fx-core/src/component/utils/envFunctionUtils.ts 89.47% 1 Missing and 1 partial ⚠️
packages/manifest/src/manifestTemplate.ts 98.21% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev   #16270      +/-   ##
==========================================
+ Coverage   86.93%   87.08%   +0.15%     
==========================================
  Files         628      629       +1     
  Lines       32732    33143     +411     
  Branches     7690     7716      +26     
==========================================
+ Hits        28454    28864     +410     
+ Misses       2440     2437       -3     
- Partials     1838     1842       +4     
Files with missing lines Coverage Δ
packages/fx-core/src/component/utils/common.ts 76.08% <ø> (-6.46%) ⬇️
...es/fx-core/src/component/utils/envFunctionUtils.ts 89.74% <89.47%> (-2.26%) ⬇️
packages/manifest/src/manifestTemplate.ts 98.21% <98.21%> (ø)

... and 77 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

iclanton added 2 commits July 6, 2026 21:55
…pp-manifest

Relocate the DriverContext-free parts of fx-core's manifest templating into
@microsoft/app-manifest so the logic can be consumed without fx-core's
DriverContext, localization, or FxError:

- expandEnvironmentVariable / getEnvironmentVariables (moved from fx-core
  component/utils/common.ts)
- the file() function resolver and file reader (moved from fx-core
  component/utils/envFunctionUtils.ts), decoupled from DriverContext and
  raising plain typed errors that carry the offending path/token
- the ManifestType enum

Also expose expandFileFunctionMacros (the resolution loop, returning the
expanded content plus a function count for host telemetry) and resolveManifest,
the host-agnostic counterpart of fx-core's getResolvedManifest. Reuses the
package's existing strip-bom and fs-extra usage (no new dependencies).

Adds unit tests covering file inlining, JSON escaping, BOM/CRLF normalization,
nested file() calls, env-as-parameter, ApiSpec skip, and every typed error path.
Now that the resolver lives in @microsoft/app-manifest (reached transitively via
@microsoft/teamsfx-api), remove the duplicated logic from fx-core and delegate:

- component/utils/common.ts re-exports expandEnvironmentVariable and
  getEnvironmentVariables from @microsoft/teamsfx-api, so existing call sites are
  unchanged.
- envFunctionUtils.expandVariableWithFunction delegates its loop to
  expandFileFunctionMacros, keeping only the telemetry event and mapping the
  resolver's plain errors to localized UserError / fx-core FileNotFoundError via
  toFxError. getResolvedManifest is unchanged and keeps composing these primitives.

Update the affected tests to drive resolution through real temp files instead of
fs mocks, since file reads now happen inside app-manifest.
@iclanton
iclanton force-pushed the odsp/manifest-api-extraction branch from 18afc6a to 23a1773 Compare July 6, 2026 21:57
@iclanton iclanton changed the title refactor: extract host-agnostic manifest template resolver into @microsoft/app-manifest refactor: move manifest template resolver into @microsoft/app-manifest Jul 6, 2026
isJson: boolean,
options: Pick<ResolveManifestOptions, "envs" | "fromPath" | "logger">
): Promise<{ content: string; functionCount: number }> {
const matches = content.match(functionRegex);
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.

2 participants