applyPostTransforms should not be applied for all builder schemas

Open
#14,827 2 comments 3 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
35/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Stale
Tech stack
typescript
Domain
tooling

Research direction

Start with packages/angular_devkit/core/src/json/schema/registry.ts, especially the applyPostTransforms option, then inspect packages/angular_devkit/architect/src/architect.ts where validation is called without a second argument. Reproduce the builder schema behavior described in the issue and determine how a specific builder can disable addUndefinedDefaults without changing existing defaults for other schemas.

Written by the indexing model from the issue text.

Description

area: @angular-devkit/architect feature feature: insufficient votes

🚀 Feature request

Command (mark with an x)
- [ ] new
- [ ] build
- [ ] serve
- [ ] test
- [ ] e2e
- [ ] generate
- [ ] add
- [ ] update
- [ ] lint
- [ ] xi18n
- [x] run
- [ ] config
- [ ] help
- [ ] version
- [ ] doc
Description

I am currently writing a builder and the appropriate JSON schema to validate its options.
The builder will be used in different targets, but all of these targets usually share some base options which is why I added some target reference support, similar to the browserTarget
option of the @angular-devkit/build-angular:dev-server builder.
An example config could look like this:

"target1": {
    "builder": "mybuilder:f":
    "options": {
        "a": "foo",
        "b": {
            "asd": "wer"
        }
    }
},
"target2": {
    "builder": "mybuilder:f":
    "options": {
        "targetRef": "myproject:target1", // References the options of target1 above
        "a": "bar"
    }
},

So, at the end the options of target2 should be evaluated to the following:

{
    "a": "bar",
    "b": {
        "asd": "wer"
    }
}

This should be as simple as using some Object.assign code:

const target2Options = {  "a": "bar"  };
const target1Options = {  "a": "foo", "b": { "asd": "wer" }  };
const result = Object.assign({}, target1Options, target2Options);

But the problem is, that Angular modifies the incoming options-object. It passes in an empty object for the b property of target2's options: { "a": "bar", "b": {} }. Thus, result.b will be {} instead of { "asd": "wer" }.
This is done by the Angular's postTransform addUndefinedDefaults which is always added to the CoreSchemaRegistry. In theory postTransforms can be disabled as seen here using applyPostTransforms: false. However, it is not possible to override this value when the builder's schema is compiled and the validator is used, see here (no second argument is passed to validation).

Of course, I could add some code to my builder that checks for that {} and ignores it. But actually, it should be possible to explicitly set b: {} to override the inherited value, e.g.:

"target3": {
    "builder": "mybuilder:f":
    "options": {
        "targetRef": "myproject:target1",
        "b": {}
    }
},

So I really need to distinguish between the {} set by the user and the {} set by the postTransform.

NOTE: All of this affects options that are set to type object or array (in which case [] is used as default) in the schema.

Describe the solution you'd like

It would be nice to be able to disable postTransforms for a specific builder, for example in the builder code itself.

Actually I do not know why the addUndefinedDefaults transform is used at all, but there is probably a reason for that. Is it? 😛

Describe alternatives you've considered

I cannot imagine any workarounds. Angular modifies the options passed to a builder where it should not, IMHO. In the above case, the user does not specified a b property, but Angular passes in an empty object for b. And currently there is no way to change that behavior, as far as I can see.

EDIT:
I did find a workaround, but it is an ugly hack as it relies on the internals of addUndefinedDefault and thus could easily break when Angular is updated.

Instead of this schema:

"b": {
    "type": "object",
    "additionalProperties": {
        "type": "string"
    }
}

One can use this schema:

"b": {
    "oneOf": [
        {
            "type": "null"
        },
        {
            "type": "object",
            "additionalProperties": {
                "type": "string"
            }
        }
    ]
}

This works, because Angular detects that two types are possible for b: objectand null. This causes Angular to not override the value.
Of course, null is now a valid value for b, too, but that may not be a problem in some cases.

Dominant language
TypeScript
Stars
27k
Forks
11.8k
Avg merge
16h 35m
Merged PRs (30d)
176

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from angular/angular-cli

All issues in angular/angular-cli

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.