Skip to content

[EngSys] Fix shared Vitest dependency resolution - #39845

Open
Jeremy Meng (jeremymeng) wants to merge 2 commits into
Azure:mainfrom
jeremymeng:jeremymeng-fix-vitest-import-warnings
Open

[EngSys] Fix shared Vitest dependency resolution#39845
Jeremy Meng (jeremymeng) wants to merge 2 commits into
Azure:mainfrom
jeremymeng:jeremymeng-fix-vitest-import-warnings

Conversation

@jeremymeng

Copy link
Copy Markdown
Member

Copilot agent :copilot: (on behalf of Jeremy Meng (@jeremymeng)): Fix shared Vitest configuration imports under pnpm's strict dependency resolution.

Packages impacted by this PR

All packages that consume the repository's shared Vitest configuration. The change is owned by the root @azure/monorepo workspace package.

Issues associated with this PR

N/A

Describe the problem that is addressed by this PR

The root vitest.shared.config.ts imports vitest/config and vitest/node, but the root package did not declare Vitest. When a leaf SDK package loaded this shared configuration, the config bundler could not resolve those imports from the root and emitted UNRESOLVED_IMPORT warnings even though tests passed.

This PR declares Vitest at the root and also declares the existing @types/node catalog dependency so pnpm reuses the repository's Node 22 Vite peer variant instead of creating a Node 25 variant.

What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

The alternatives were to duplicate or redirect Vitest imports in each leaf package, or change config loading to tolerate unresolved imports. Declaring the dependencies where the shared configuration lives preserves strict resolution and fixes all consumers without per-package changes.

Are there test cases added in this PR? (If not, why?)

No new test case is needed because this is a dependency-resolution correction. The original @azure/storage-queue command was run and completed with 21 passing tests without either UNRESOLVED_IMPORT warning:

npm run test:node -- -- .\test\queueclient.spec.ts

Provide a list of related PRs (if any)

N/A

Command used to generate this PR:**(Applicable only to SDK release request PRs)

N/A

Checklists

  • Added impacted package name to the issue description
  • Does this PR need any fixes in the SDK Generator? (If so, create an Issue in the typespec-azure repository and link it here)
  • Added a changelog (if necessary)

Copilot AI balanced review requested due to automatic review settings September 2, 2026 21:35
@jeremymeng
Jeremy Meng (jeremymeng) requested a review from a team as a code owner September 2, 2026 21:35
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
9 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

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.

🟢 Approval recommended

The reviewed change resolves dependency imports with no unresolved issues.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview

Declares shared Vitest dependencies at the workspace root for strict pnpm resolution.

Changes:

  • Adds catalog-managed vitest and @types/node development dependencies.
File summaries
File Description
package.json Declares dependencies required by the shared Vitest configuration.
Review details

Files excluded by content exclusion policy (1)

  • pnpm-lock.yaml
  • Files reviewed: 1/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

Copilot AI review requested due to automatic review settings September 3, 2026 00:35

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.

🟢 Approval recommended

The reviewed change resolves dependency ownership with no unresolved issues.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Review details

Files excluded by content exclusion policy (1)

  • pnpm-lock.yaml
  • Files reviewed: 1/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

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