Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[heft-node/web-rig] Update the rush-project.json file to follow the new schema and configure the "build" command output folders for the "_phase:build" phase. #3144

Merged
merged 9 commits into from
Jan 19, 2022

Conversation

iclanton
Copy link
Member

@iclanton iclanton commented Jan 7, 2022

Adds a _phase:build output entry to rush-project.json in the rigs.

@iclanton iclanton enabled auto-merge January 7, 2022 03:17
@elliot-nelson
Copy link
Collaborator

elliot-nelson commented Jan 7, 2022

🤔 Extrapolating from this change... if you haven't phase-ified your monorepo, this is using the synthetic phase generated for a bulk action?

So if I make a custom bulk action foo, can I then set up caching for it just by setting up _phase:foo, even if I keep foo a bulk action? That would be ideal.

(Clarification: I never configure a "_phase:foo", but I put "_phase:foo" in my rig rush-project.json).

@iclanton
Copy link
Member Author

iclanton commented Jan 8, 2022

The synthetic phase names actually don't have the _phase: prefix, so a bulk action called foo gets translated internally to a phased command with a single phase also called foo. We don't actually validate that the phase names in rush-project.json have the prefix, so you could actually cache a bulk action called foo with a "phaseName:" "foo" in a rush-project.json's phaseOptions.

We do validate that the phases are defined in the repo, which is why this PR's build is failing. I think we should consider either eliminating that requirement or relaxing it. The alternative is that we have to put together an internal rig that has a rush-project.json with the _phase:build's phase options in it. @octogonz - thoughts?

@elliot-nelson
Copy link
Collaborator

I think what's bothering me is that this _phase:build folder list is almost certainly wrong, right?

For example, if you have a bog-standard "heft+jest" compilation setup and you wanted to enable phased builds with a bare minimum of phases, then the configuration changes you'd want to make would be:

(1) Create _phase:build and _phase:test.
(2) Probably build is heft build --clean and test is heft test --no-build.
(3) Set up phaseOptions where _phase:build cache folders are lib,lib-commonjs,dist and _phase:test cache folders are temp. (Because temp is where you put stuff like temp/coverage and temp/jest-reports, you don't typically write to it from build.)

I think maybe, rather than trying to put a "default" _phase:build in the rig, you should put a "default" build there instead, for monorepos that haven't converted their build from a bulk action yet.

Then when the monorepo maintainer converts their build to a phased build, and defines the phases, when they attempt to run the build it should error and force them to define the phase-specific build folders, rather than having a default.

@elliot-nelson
Copy link
Collaborator

elliot-nelson commented Jan 11, 2022

Ohhhhh....

I think I misunderstood the problem. Regardless of my suggestion above, you're pointing out that if Rush itself uses the default rig, then Rush can't convert to phased builds without breaking the default rig. 🤔

@iclanton
Copy link
Member Author

The issue is that the code that loads rush-project.json has a check to see if output folders are defined for any phases that aren't defined in the repo. For example, if we're in a repo with _phase:build and _phase:test configured in common/config/rush/command-line.json and a project has a config/rush-project.json that configures output folders for _phase:lint, Rush will error when loading that rush-project.json file because it doesn't know what _phase:lint is.

This breaks down if a rig defines options for phases that a repo doesn't use, or if the repo hasn't opted into phased builds.

@iclanton iclanton changed the title [heft-node/web-rig] Add _phase:build configurations to rush-project in rigs. [heft-node/web-rig] Update the rush-project.json file to follow the new schema and configure the "build" command output folders for the "_phase:build" phase. Jan 18, 2022
@iclanton iclanton disabled auto-merge January 19, 2022 01:57
@iclanton iclanton requested a review from patmill as a code owner January 19, 2022 01:59
@iclanton iclanton enabled auto-merge January 19, 2022 02:04
… version in rush.json is ahead of the version in Rush's package.json
@iclanton iclanton merged commit 3292dad into microsoft:master Jan 19, 2022
@iclanton iclanton deleted the phasify-rigs branch January 19, 2022 02:58
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.

3 participants