Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request improves error messages when running schematics that require a project in a multi-project workspace (or a workspace with no projects) where the project cannot be determined from the current working directory. It adds clear guidance on using the --project option and lists available projects, supported by new unit and end-to-end tests. The feedback suggests guarding the cleanup in afterAll within the unit tests to prevent a secondary TypeError if beforeAll fails and root is undefined.
There was a problem hiding this comment.
Thanks for looking into this. However, intercepting SchemaValidationException in the catch block and matching raw AJV error properties for missingProperty === 'project' is pretty brittle:
- Because
getMissingProjectMessagereplaces the entire error message whenerror.errors.some(...)matchesproject, any other schema validation errors from the same run (such as invalid flag values or other missing required properties) are completely swallowed. - It assumes that any top-level required
projectproperty in any schematic (including third-party schematics or nested schematics called viaexternalSchematic) uses"$default": { "$source": "projectName" }. - Because this only runs after prompt evaluation and schema validation fail, running
ng g cinteractively in a multi-project workspace will first prompt the user for the component name and only fail with the missing project error after they answer. - The E2E test addition in
project-cannot-be-determined-by-cwd.tsis good, but the unit test inschematics-command-module_spec.tsis overly complex: it scaffolds a fake schematic collection on disk (usingos.tmpdir()instead ofprocess.env['TEST_TMPDIR'], which breaks Bazel sandboxing rules), spins up a fullNodeWorkflow, and implicitly relies onprocess.cwd()not falling under the tempworkspace.basePath.
If we want to handle missing projectName smart defaults cleanly, this should be tied to the projectName smart default / schema option handling itself rather than scraping AJV validation errors in catch.
|
Thanks for catch! Reworked it in 061e5c5
Didnt put it inside |
…not determine one when project can't be resolved from cwd, like at root of workspace with no root project, schematics fail with raw `Data path "" must have required property 'project'` from schema validation now that error is replaced with message that says to pass `--project` or cd into project and lists available projects like architect commands do, empty workspace gets hint to generate application or libary first Fixes angular#25402
061e5c5 to
b7f1e26
Compare
PR Checklist
PR Type
What is the current behavior?
Issue Number: #25402
My library ngwr keeps
libandshowcaseunderprojects/with no root project, andng g c foofrom workspace root on 22.2.3 gives:projectNamesmart default comes fromgetProjectByCwd(), it returns nothing when cwd is outside every project or inside two projects with same root, soprojectstays empty and ajv error goes to user as isArchitect commands already explain this with
Cannot determine project for command, schematics didntWhat is the new behavior?
When schematic fails because required
projectis missing and CLI could not resolve it, error says what to do, as Alan suggested in issue:Workspace with no projects gets
This workspace does not contain any projects. Run "ng generate application" or "ng generate library" to create one.instead of empty listAny other validation error is printed same as before
New
schematics-command-module_spec.tsruns real workflow with small local collection that requiresproject, both cases fail on mainAlso added
ng generate componenttoproject-cannot-be-determined-by-cwd.tse2e, it already sets up two apps without root project and prints same ajv error on mainDoes this PR introduce a breaking change?
Other information
Check sits in
catchofrunSchematicbecauseprojectcan also come fromschematicsoptions in angular.json, and validation is only place that knows it is really missing. Can move it before workflow runs, likegetProjectNamesByTargetdoes for architect, if you prefer thatBuilt CLI from this branch and ran
ng g c foo --dry-runat ngwr root, it prints message above, and with--project showcaseor fromprojects/showcaseit works as beforecloses #25402