[rush] Issue: API Extractor output is an untracked build output

[elliot-nelson]

Summary

Although not a critical issue, we ran into some confusion this week because auto-generated api extractor output is a build output that is not tracked by the build cache.

Repro steps

To reproduce this issue:

• A local developer touches Project A and adds a method and runs rush build
• This updates the common/api-reviews folder
• After consideration, they remove the method and add it to Project B instead, and run rush build
• This updates common/api-reviews folder for Project B, but not Project A

The developer now puts up a PR, which contains only change -- a new method on Project B -- but the api-reviews folder contains that method in both Project A and Project B.

The fix is easy -- go and run "heft build" manually in the project to rebuild it, which updates api-reviews folder again, and push the change.

Details

I don't think this is a "bug", but I am raising it to track in future conversations about how build caching works. Resetting a project back to a previous version and running rush build doesn't guarantee that untracked outputs are reset.

API Extractor reviews are one example. Another example is heft/jest snapshots (not quite as bad, because rush build itself can't update snapshots unless the monorepo maintainer has created a custom bulk command to do so).

In some ways, both API Extractor reviews and Jest snapshots are "tracked tests" of a build output. But it seems like a flaw, or maybe a missing safety feature, that a multi-project rush watch mode (as an example) can update a bunch of tracked tests, but then changing the code back just leaves those new changes sitting around and can't reset them back.

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question	Answer
@microsoft/rush globally installed version?
rushVersion from rush.json?
useWorkspaces from rush.json?
Operating system?
Would you consider contributing a PR?
Node.js version (node -v)?

[elliot-nelson]

I ran into this issue again with a different developer, slightly simpler scenario:

• Developer added new method on Project A (generating api review report)
• Developer needed to work on different bug, so they git add "." (current project), save index, go to another branch
• While working on another bug, they threw away unstaged changes
• Developer returned to original branch, and prepared to push; running "rush build --to ." pulled from cache and did not replace file
• CI failed every time, but local build worked

The fix is easy -- rush rebuild -- if you know what the problem is.

I think the solution here may be that "validate API Extractor report is valid" should be a different phase then "generate API Extractor report".

• _phase:build produces lib/ and temp/api-extractor (cached)
• _phase:api_extractor compares temp/api-extractor to common/api-reviews, either producing warning or copying file(s), depending on --production flag (never cached)

This increases the total build time a little, but would always produce the expected result.

[elliot-nelson]

I was thinking about this again because we had another occurrence in our monorepo.

I think there's two viable solutions here:

• One, like suggested above, would be to split out "verification of api extractor output" as a separate never-cached phase, which could then protect your PR build from any potential unwanted differences.
• Another option would be to add a missing feature to the caching. In addition to outputFolderNames, we support additionalOutputFiles, an array of file globs that can live inside or outside the repo. Each glob string would be evaluated, all of the files gathered up into a temporary folder with a metadata file describing each file and its intended target location, and then the whole thing would be zipped up and cached behind the scenes. Restoring an "additionalOutputFile" would mean generating a base64 encode of the glob, looking up the cache entry, unzipping it, and then copying each file into its target location (which could be any file in the repo, e.g. common/api-reviews/xyz.md) or any file outside the repo.

Perhaps the second option is too powerful/dangerous, but it seems like the more direct solution for the problem, which is that we have a file produced by a build step that isn't in the cached outputs list.

[dmichon-msft]

For the validation case, you can use the dependsOnAdditionalFiles property to indicate that the validator project depends on the file in common/api-reviews, and the cache entry will account for the state of the file (so it could be a cached phase, you don't have to disable). Allowing Rush to track file writes to locations outside of the repository gets dangerous, mostly because we don't really want to be in the business of allowing tarballs to unpack beyond the scope of their root directory.

[elliot-nelson]

@dmichon-msft Makes sense.

In our Rush repo, the userbase has grown significantly and this issue was occurring a lot and causing quite a bit of confusion, so we ended up moving the reviews into the project folders themselves.

• In api-extractor.json: "reportFolder": "<projectFolder>/api-reviews/"
• In rush-project.json: added api-reviews to output folders for build phase

This way it is just treated as a regular build output and can't get out-of-date (or at least, it can't due to build cache behavior).