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
Sorry, something went wrong.
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 GitHubYour 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".
Sorry, something went wrong.
|
rescript
npm i https://pkg.pr.new/rescript@8540
npm i https://pkg.pr.new/@rescript/darwin-arm64@8540
npm i https://pkg.pr.new/@rescript/darwin-x64@8540
npm i https://pkg.pr.new/@rescript/linux-arm64@8540
npm i https://pkg.pr.new/@rescript/linux-x64@8540
npm i https://pkg.pr.new/@rescript/runtime@8540
npm i https://pkg.pr.new/@rescript/win32-x64@8540 commit: a8a69e0 |
Sorry, something went wrong.
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 GitHubYour 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".
Sorry, something went wrong.
| source_files | ||
| .keys() |
There was a problem hiding this comment.
Remove outputs for sources missing from discovery
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 👍 / 👎.
Sorry, something went wrong.
| 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.
Migrate compiler-info files from the previous schema
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 👍 / 👎.
Sorry, something went wrong.
Signed-off-by: Christoph Knittel <ck@cca.io>
|
@jfrolich could you have a look? I think the Codex findings above are edge cases that we can skip. |
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
Summary
Rewatch now records the effective package output settings in each package's compiler-info.json. When the consuming project's module format, in-source setting, 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