Rebuild source dependencies when package output settings change - #8540
Rebuild source dependencies when package output settings change#8540cknitt wants to merge 1 commit into
Conversation
dc1214d to
a33b3b4
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a33b3b4855
ℹ️ 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".
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
a33b3b4 to
0bfbbbd
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bfbbbd36c
ℹ️ 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".
| source_files | ||
| .keys() |
There was a problem hiding this comment.
When a dependency loses a source file in the same update that changes the root package specs, source_files contains only the dependency's current files, so the previous output for the removed module is never visited here. Verification subsequently calls clean_package, deleting the old AST/compiler state before cleanup_previous_build can discover that removed module; the build then emits the remaining modules under the new spec but leaves the removed module's old .cjs/.mjs output behind. Use the previous build manifest or scan the previous output locations rather than limiting removal to current source keys.
Useful? React with 👍 / 👎.
| let parsed: Result<CompilerInfoFile, _> = serde_json::from_str(&contents); | ||
| let parsed = match parsed { | ||
| Ok(p) => p, | ||
| Err(_) => return true, // unknown or invalid format -> treat as mismatch | ||
| Err(_) => return Some((package, None)), // unknown or invalid format -> treat as mismatch |
There was a problem hiding this comment.
When the first build with this version both upgrades Rewatch and changes package-specs, every compiler-info.json written by the previous version lacks the newly required package_output_specs field, so deserialization deterministically takes this branch and records no previous specs. The later cleanup therefore skips generated-output removal while deleting compiler assets, and recompilation leaves the old suffix or output directory alongside the new one. Fresh evidence beyond the earlier stale-output comment is the newly required schema field, which makes this path unavoidable for all pre-change metadata; deserialize it as optional or perform conservative output cleanup during migration.
Useful? React with 👍 / 👎.
Signed-off-by: Christoph Knittel <ck@cca.io>
0bfbbbd to
a8a69e0
Compare
1 participant
Summary
Rewatch now records the effective package output settings in each package's
compiler-info.json. When the consuming project's module format,in-sourcesetting, or resolved suffix changes, affected source dependencies are cleaned and rebuilt.This follows the existing invalidation mechanism for root source-map arguments.
A regression test covers rebuilding a shared source dependency after switching from CommonJS to ES modules.
Fixes #8539