Skip to content

fix(deps): resolve missing source-imported packages from the component model on import#10365

Draft
zkochan wants to merge 2 commits into
teambit:masterfrom
zkochan:fix-missing-deps-from-model
Draft

fix(deps): resolve missing source-imported packages from the component model on import#10365
zkochan wants to merge 2 commits into
teambit:masterfrom
zkochan:fix-missing-deps-from-model

Conversation

@zkochan
Copy link
Copy Markdown
Member

@zkochan zkochan commented May 12, 2026

Summary

  • When bit import brings 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 to missing.packages. processMissing then surfaces a MissingPackagesDependenciesOnFs issue — even though the imported component's model already records the package in packageDependencies.
  • processPackages already falls back to componentFromModel for resolved packages; this PR adds the same fallback in processMissing. If a "missing" package is listed in the model, move it into tree.packages with the model's recorded version so it enters the workspace manifest and gets installed by bit import — no follow-up bit install --add-missing-deps needed.
  • Truly-missing packages (not in workspace policy and not in the model) still surface as before.

Test plan

  • New e2e test e2e/harmony/import-resolves-missing-deps-from-model.e2e.ts covers the regression: a component requiring is-positive is tagged & exported, then imported in a fresh workspace; asserts that MissingPackagesDependenciesOnFs is not reported.
  • Verified the test fails on the prior code (issue surfaces) and passes with this fix.

…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.
Copilot AI review requested due to automatic review settings May 12, 2026 11:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 into tree.packages when they exist in componentFromModel.getAllPackageDependencies(), preventing missing-package issues for those packages.
  • Add an E2E regression test that imports a component requiring is-positive into 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
Copy link
Copy Markdown
Member Author

zkochan commented May 12, 2026

Converting to draft. Verified against bit lane import bitdev.react/fix-react-peers-conflicts-full-scope that this PR is not required for that scenario — #10366 alone fixes it. Keeping this open as a narrower, independent correctness fix: in cases where a missing source-imported package is recorded in the component's model but on-disk resolution fails and it isn't installed via the scope's dependenciesGraph, this lets auto-detect treat it as resolved from the model. Not a release blocker; revisit when convenient.

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.
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