Rewatch: feature-gated source directories#8379
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 767aca324e
ℹ️ 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".
| if !any_all_request && requested.is_empty() { | ||
| any_all_request = true; |
There was a problem hiding this comment.
Preserve empty feature lists instead of expanding to all features
This fallback treats an empty requested set as all features, which breaks the qualified dependency form when a consumer explicitly sets "features": []. In that case no features are requested, but requested remains empty and this branch forces any_all_request = true, enabling all feature-tagged source dirs for that dependency. That makes it impossible to express an untagged-only dependency build despite using the restrictive object form.
Useful? React with 👍 / 👎.
Add a `feature` tag on source entries so a package can ship optional
slices of its source tree that consumers opt into. The new top-level
`features` map declares transitive implications, and `dependencies` /
`dev-dependencies` accept an object form `{name, features}` so a
consumer can restrict which features of a dep get built.
On the CLI, `rewatch build --features=a,b` (and the same on `watch`)
restricts the current package; dependencies inherit from consumer
declarations in rescript.json (union across consumers). Omitting the
flag keeps all features active.
Filtering runs inside `packages::make` before source files are
discovered, so the watcher automatically skips disabled directories and
the existing diff-based cleanup removes orphan artifacts when a feature
is toggled off. `clean` intentionally ignores `--features` so it
always wipes the full tree.
Cycles in the `features` map are rejected at load time with a message
naming the participants. Empty `--features=` is rejected with guidance
to omit the flag instead.
Includes unit tests for parsing, closure resolution, cycle detection,
and per-package active-set computation; six bash integration tests
covering CLI restriction, transitive expansion, artifact cleanup on
toggle, cycle errors, and empty-flag rejection. Documentation in
rewatch/Features.md and a row added to
rewatch/CompilerConfigurationSpec.md.
Signed-off-by: Jaap Frolich <jaap@tella.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Jaap Frolich <jfrolich@gmail.com>
compute_active_features scanned both dependencies and dev-dependencies on every consumer regardless of the active build mode. In --prod, a dep reached via dependencies with a restricted feature list could still have a shorthand dev-dependencies entry on the same consumer flip any_all_request = true and force all features active — pulling in feature-gated code that --prod is supposed to exclude. Match read_dependencies's rule: dev-dependency edges only contribute to a dep's active feature set when the consumer is local and we're not in --prod. Regression test covers both modes. Signed-off-by: Jaap Frolich <jaap@tella.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Jaap Frolich <jfrolich@gmail.com>
36b09f3 to
025f146
Compare
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
compute_active_features conflated "no consumer edge found" with "consumer requested empty". A qualified dependency with `"features": []` would land in the same state as an unreached dep (requested set empty, no `any_all_request`) and trip the fallback that forced all features active. That made it impossible to express an untagged-only dependency build. Track `saw_consumer_entry` explicitly. The all-features fallback now fires only when no consumer edge was observed; an empty requested set from an explicit `"features": []` is honoured as "no feature-gated dirs". Regression test covers the explicit-empty case. Docs updated. Signed-off-by: Jaap Frolich <jaap@tella.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Jaap Frolich <jfrolich@gmail.com>
025f146 to
6de16a6
Compare
Summary
featuretag onsourcesentries so packages can ship optional slices of their source tree. A top-levelfeaturesmap declares transitive implications, anddependencies/dev-dependenciesaccept a new object form{name, features}so consumers can restrict which features of a dep get built.rewatch build --features=a,b(andwatch) restricts the current package. Dependencies inherit from consumer declarations in rescript.json; multiple consumers union. Omitting the flag keeps all features active. Empty--features=is rejected with guidance.packages::makebefore source discovery, so the watcher naturally skips disabled dirs and the existing diff-based cleanup removes orphan artifacts when a feature is toggled off.cleanintentionally ignores--featuresand wipes the full tree.featuresmap fail fast at load time with a message naming the participants.rewatch/Features.md(dedicated), plus a row inrewatch/CompilerConfigurationSpec.md.Why
Lets a library author gate backend- or platform-specific code behind an opt-in flag without paying the compile cost for consumers who don't need it — and lets consumers restrict heavy deps to only the features they use.
Test plan
cargo build --manifest-path rewatch/Cargo.tomlcargo clippy --manifest-path rewatch/Cargo.toml --all-targets --all-features— no warningscargo fmt --check --manifest-path rewatch/Cargo.toml— cleancargo test --manifest-path rewatch/Cargo.toml --lib— 89 passing (21 new)make test-rewatch— full suite including 6 new feature tests underrewatch/tests/features/(104 assertions pass, 0 fail)Open follow-ups (not in this PR)
rescript-editor-analysis) doesn't yet know aboutfeatures. Feature-gated dirs are still visible to the LSP, so users can jump-to-def into code that won't compile under their current feature set — same UX astype: "dev"today.dependencies(as opposed to feature-gated source dirs within a dep) are out of scope.🤖 Generated with Claude Code