fix(deps): resolve missing source-imported packages from the component model on import - #10365
Merged
Merged
Conversation
…t model on import When a freshly imported component requires a package that isn't yet in node_modules, the dependency tree walker put it in missing.packages and auto-detect surfaced MissingPackagesDependenciesOnFs. processPackages already falls back to componentFromModel for resolved packages, but processMissing didn't. Apply the same fallback: if the model records the package in packageDependencies, move it into tree.packages with the model's version so it enters the workspace manifest and gets installed during bit import — no need for a follow-up bit install --add-missing-deps.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a false-positive MissingPackagesDependenciesOnFs issue during bit import when the imported component’s model already records the required package dependency, but the importing workspace’s node_modules is still “cold” at dependency-walk time. The change makes processMissing fall back to componentFromModel (similar to the existing behavior in processPackages) so model-recorded packages are treated as resolved and flow into the dependency tree.
Changes:
- In
AutoDetectDeps.processMissing, move “missing” packages intotree.packageswhen they exist incomponentFromModel.getAllPackageDependencies(), preventing missing-package issues for those packages. - Add an E2E regression test that imports a component requiring
is-positiveinto a fresh workspace (with--skip-dependency-installation) and asserts the missing-package issue is not reported.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
scopes/dependencies/dependencies/dependencies-loader/auto-detect-deps.ts |
Adds model-based fallback for packages initially classified as “missing” to avoid incorrect MissingPackagesDependenciesOnFs reporting on import. |
e2e/harmony/import-resolves-missing-deps-from-model.e2e.ts |
Regression test covering import behavior when the model already lists a required package dependency. |
zkochan
marked this pull request as draft
May 12, 2026 14:01
Member
Author
|
Converting to draft. Verified against |
zkochan
added a commit
that referenced
this pull request
May 12, 2026
…resolvePackageNameByPath (#10366) ## Summary `pathNormalizeToLinux` (via `normalize-path`) already strips trailing slashes, but the single-segment branch in `resolvePackageNameByPath` returned the *original* (un-normalized) `packagePath`. So inputs like `events/` leaked through unchanged. That broke downstream matching: `MissingPackagesDependenciesOnFs` reported the package as `events/`, while every other consumer — workspace policy lookups, `resolvedPackageData.name` from `resolvePackageData`, and the component model's `packageDependencies` — uses the canonical name `events`. The trailing-slash form appears in real-world code (most commonly webpack browser-fallback configs: `require.resolve('events/')`), where it's used to force resolution to the npm `events` polyfill rather than Node's builtin. Returning the normalized path fixes the leak and aligns all consumers on the canonical package name. This is part of fixing the `bit lane import` symptom where a webpack-bundler-style component reports `config/webpack-fallbacks.ts -> events/` as a missing package: this PR makes the missing package surface as `events` instead of `events/`. Together with #10365 (which lets `processMissing` fall back to the component model when the model already records the dep), the issue clears cleanly on import. ## Test plan - [x] New e2e test `e2e/harmony/missing-package-strips-trailing-slash.e2e.ts` asserts the `MissingPackagesDependenciesOnFs` data records `events`, not `events/`, for `require('events/')`. - [x] Verified the test fails on the prior code (`expected [ 'events/' ] to include 'events'`) and passes with the fix.
zkochan
marked this pull request as ready for review
September 1, 2026 12:28
GiladShoham
approved these changes
Sep 1, 2026
zkochan
enabled auto-merge (squash)
September 1, 2026 12:29
PR Summary by QodoResolve imported package dependencies from the component model
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1. Standalone import E2E duplicates area
|
![qodo-free-for-open-source-projects[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| // the bug by hoisting the package via the scope's dependenciesGraph) doesn't run. That makes | ||
| // the auto-detect path the only thing deciding whether the issue is reported, which is exactly | ||
| // the path the fix targets. | ||
| describe('bit import: missing-package issue suppressed when the model lists the package', function () { |
There was a problem hiding this comment.
1. Standalone import e2e duplicates area 📘 Rule violation ➹ Performance
The PR creates a standalone E2E file even though import-harmony.e2e.ts already covers import-without-install scenarios using the same workspace lifecycle. This unnecessarily fragments the existing import test area and adds separate E2E setup overhead.
Agent Prompt
## Issue description
The regression scenario was added as a new E2E file despite an existing E2E suite dedicated to Harmony import behavior.
## Issue Context
The scenario genuinely exercises workspace import behavior, but Compliance Rule 2 requires necessary E2E coverage to be added to an existing relevant file where possible. Move this focused scenario into `import-harmony.e2e.ts` and remove the standalone file while preserving its assertions and cleanup behavior.
## Fix Focus Areas
- e2e/harmony/import-resolves-missing-deps-from-model.e2e.ts[1-40]
- e2e/harmony/import-harmony.e2e.ts[164-175]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+523
to
+524
| if (this.componentFromModel) { | ||
| const modelDeps = this.componentFromModel.getAllPackageDependencies(); |
There was a problem hiding this comment.
2. Peer model dependencies remain missing 🐞 Bug ≡ Correctness
The new fallback checks only getAllPackageDependencies(), which excludes peerPackageDependencies, so source imports of model-recorded peers remain in missing.packages and still raise MissingPackagesDependenciesOnFs. This prevents the later peer-classification logic from recovering the dependency because it only reclassifies packages already detected as runtime or dev dependencies.
Agent Prompt
## Issue description
The missing-package fallback only consults `componentFromModel.getAllPackageDependencies()`, which includes runtime and dev dependencies but excludes peer package dependencies. An imported component requiring a model-recorded peer can therefore still receive `MissingPackagesDependenciesOnFs`.
## Issue Context
Peer dependencies are a separate model category. Once a package is placed into the detected runtime/dev package map, the existing override pipeline can reclassify it as a peer using the component model's package metadata.
## Fix Focus Areas
- scopes/dependencies/dependencies/dependencies-loader/auto-detect-deps.ts[523-530]
- components/legacy/consumer-component/consumer-component.ts[329-331]
- scopes/dependencies/dependencies/dependencies-loader/apply-overrides.ts[458-480]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Code Review by Qodo
1. Model peers remain missing
|
| // model's version so they end up in the workspace manifest and get installed — rather than | ||
| // surfacing as MissingPackagesDependenciesOnFs issues. | ||
| if (this.componentFromModel) { | ||
| const modelDeps = this.componentFromModel.getAllPackageDependencies(); |
There was a problem hiding this comment.
1. Model peers remain missing 🐞 Bug ≡ Correctness
The fallback uses getAllPackageDependencies(), which excludes peerPackageDependencies, so a source-imported peer absent from node_modules remains in missing.packages and raises MissingPackagesDependenciesOnFs. It also never enters detected packages, preventing applyPeersFromComponentModel() from restoring its peer classification and version.
Agent Prompt
## Issue description
Include model-recorded peer package dependencies when reconciling missing source imports, so valid peers do not remain missing on cold imports.
## Issue Context
`getAllPackageDependencies()` merges only runtime and dev package dependencies. Peer reclassification later operates only on packages already detected, so the fallback must seed model peers into `tree.packages` as well. Add regression coverage for a component that imports a model-recorded peer with an empty `node_modules`.
## Fix Focus Areas
- scopes/dependencies/dependencies/dependencies-loader/auto-detect-deps.ts[523-530]
- components/legacy/consumer-component/consumer-component.ts[329-331]
- scopes/dependencies/dependencies/dependencies-loader/apply-overrides.ts[458-480]
- e2e/harmony/import-resolves-missing-deps-from-model.e2e.ts[16-39]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bit importbrings in a component whose source requires a package, the dependency-tree walker can't resolve that package on disk (cold node_modules) and pushes it tomissing.packages.processMissingthen surfaces aMissingPackagesDependenciesOnFsissue — even though the imported component's model already records the package inpackageDependencies.processPackagesalready falls back tocomponentFromModelfor resolved packages; this PR adds the same fallback inprocessMissing. If a "missing" package is listed in the model, move it intotree.packageswith the model's recorded version so it enters the workspace manifest and gets installed bybit import— no follow-upbit install --add-missing-depsneeded.Test plan
e2e/harmony/import-resolves-missing-deps-from-model.e2e.tscovers the regression: a component requiringis-positiveis tagged & exported, then imported in a fresh workspace; asserts thatMissingPackagesDependenciesOnFsis not reported.