-
Notifications
You must be signed in to change notification settings - Fork 12k
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
feat(@angular/cli): handle string key/value pairs, e.g. --define #28362
Conversation
Unfortunately it seems like this is currently still not properly triggering usage errors when invalid values are passed. Need to dig a bit more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a couple of NITs.
packages/angular/cli/src/command-builder/utilities/json-schema.ts
Outdated
Show resolved
Hide resolved
packages/angular/cli/src/command-builder/utilities/json-schema.ts
Outdated
Show resolved
Hide resolved
packages/angular/cli/src/command-builder/utilities/json-schema.ts
Outdated
Show resolved
Hide resolved
packages/angular/cli/src/command-builder/utilities/json-schema.ts
Outdated
Show resolved
Hide resolved
9727ff8
to
7c1c955
Compare
packages/angular/cli/src/command-builder/utilities/json-schema.ts
Outdated
Show resolved
Hide resolved
packages/angular/cli/src/command-builder/utilities/json-schema.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, two minor optional nits.
Thanks for the review! |
We recently introduced the ability to pass object values from the command line (angular#28362). @clydin noticed that the initial behavior didn't work well for `--define`: It completely replaced all values even if just one of multiple defines is specified. This updates the architect to support merging of object options. If both the base option (e.g. from `angular.json`) and the override (e.g. from a CLI `--flag`) are objects, the objects are merged. See: angular#28362
We recently introduced the ability to pass object values from the command line (#28362). @clydin noticed that the initial behavior didn't work well for `--define`: It completely replaced all values even if just one of multiple defines is specified. This updates the architect to support merging of object options. If both the base option (e.g. from `angular.json`) and the override (e.g. from a CLI `--flag`) are objects, the objects are merged. See: #28362
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
PR Checklist
Please check to confirm your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The values of
define
can only be updated by changing the Angular config file.Issue Number: #4318 (arguably)
What is the new behavior?
Defines can now be introduced or changed by passing them to the CLI, e.g.:
Does this PR introduce a breaking change?
Other information
The PR also moves
addSchemaOptionsToCommand
to be colocated withparseJsonSchemaToOptions
. The two functions are tightly coupled and it makes unit testing the schema-to-yargs flow a lot easier.