Skip to content

Resolve workspace dependencies in editor analysis#8392

Merged
cknitt merged 2 commits intorescript-lang:masterfrom
cknitt:analysis-dependency-resolution
Apr 28, 2026
Merged

Resolve workspace dependencies in editor analysis#8392
cknitt merged 2 commits intorescript-lang:masterfrom
cknitt:analysis-dependency-resolution

Conversation

@cknitt
Copy link
Copy Markdown
Member

@cknitt cknitt commented Apr 27, 2026

(This was blocking #7525.)

Summary

Editor analysis now resolves dependencies from the package graph that the build system records in lib/bs/.sourcedirs.json. This makes analysis follow the same resolved dependency roots as compilation for workspace/package-manager layouts.

The existing node_modules lookup remains as a fallback for dependencies that are not present in .sourcedirs.json.

Motivation

Workspace dependencies can compile successfully without a package-local node_modules/<dependency> entry. Before this change, editor analysis still depended on that node_modules layout, so it could skip dependency modules that the build had already resolved.

That mismatch can break completions, hovers, type expansion, and other analysis features even when the project itself builds. Reading the build system's resolved package roots removes that mismatch.

This also avoids reimplementing package-manager-specific workspace resolution in analysis. The build system already knows the dependency roots and writes them to .sourcedirs.json; analysis now consumes that information.

Testing

  • make -C tests/analysis_tests/tests-sourcedirs-dependency test

@cknitt cknitt requested review from cristianoc and zth April 27, 2026 08:20
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 27, 2026

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8392

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8392

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8392

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8392

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8392

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8392

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8392

commit: 58ef19c

@cknitt
Copy link
Copy Markdown
Member Author

cknitt commented Apr 27, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Collaborator

@cristianoc cristianoc left a comment

Choose a reason for hiding this comment

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

Legit. There's code to read .sourcedirs.json in reanalyze too -- not sure if worth sharing as written in different style.

@cknitt cknitt force-pushed the analysis-dependency-resolution branch from 58ef19c to 56ea128 Compare April 28, 2026 12:43
@cknitt cknitt enabled auto-merge (squash) April 28, 2026 12:52
@cknitt cknitt merged commit 3057278 into rescript-lang:master Apr 28, 2026
25 checks passed
@cknitt cknitt deleted the analysis-dependency-resolution branch April 28, 2026 16:42
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