Skip to content

Commit

Permalink
fix(@angular-devkit/schematics): throw more relevant error when Rule …
Browse files Browse the repository at this point in the history
…returns invalid null value

A `null` value is not considered a valid return value for a schematics Rule. While the Rule
type should prevent this, casting could allow this to potentially occur. Previously, this
would accidentally be treated the same as a void return due to incomplete result checking.
However, recent refactoring caused the `null` case to fail with a non-obvious error message
when it should have failed with the existing `InvalidRuleResultException`. Non-tree result
objects including `null` will now fail with `InvalidRuleResultException`.

(cherry picked from commit 8eb58bd)
  • Loading branch information
clydin committed Oct 5, 2022
1 parent a4f4b33 commit 17eb20c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 26 deletions.
2 changes: 1 addition & 1 deletion packages/angular_devkit/schematics/src/rules/call.ts
Expand Up @@ -91,7 +91,7 @@ async function callRuleAsync(rule: Rule, tree: Tree, context: SchematicContext):
result = await result.pipe(defaultIfEmpty(tree)).toPromise();
}

if (TreeSymbol in result) {
if (result && TreeSymbol in result) {
return result as Tree;
}

Expand Down
36 changes: 11 additions & 25 deletions packages/angular_devkit/schematics/src/rules/call_spec.ts
Expand Up @@ -20,11 +20,11 @@ import {
callSource,
} from './call';

const context: SchematicContext = ({
const context: SchematicContext = {
engine: null,
debug: false,
strategy: MergeStrategy.Default,
} as {}) as SchematicContext;
} as {} as SchematicContext;

describe('callSource', () => {
it('errors if undefined source', (done) => {
Expand Down Expand Up @@ -95,34 +95,20 @@ describe('callSource', () => {
});

describe('callRule', () => {
it('errors if invalid source object', (done) => {
const tree0 = observableOf(empty());
it('should throw InvalidRuleResultException when rule result is non-Tree object', async () => {
const rule0: Rule = () => ({} as Tree);

callRule(rule0, tree0, context)
.toPromise()
.then(
() => done.fail(),
(err) => {
expect(err).toEqual(new InvalidRuleResultException({}));
},
)
.then(done, done.fail);
await expectAsync(callRule(rule0, empty(), context).toPromise()).toBeRejectedWithError(
InvalidRuleResultException,
);
});

it('errors if Observable of invalid source object', (done) => {
const tree0 = observableOf(empty());
const rule0: Rule = () => observableOf({} as Tree);
it('should throw InvalidRuleResultException when rule result is null', async () => {
const rule0: Rule = () => null as unknown as Tree;

callRule(rule0, tree0, context)
.toPromise()
.then(
() => done.fail(),
(err) => {
expect(err).toEqual(new InvalidRuleResultException({}));
},
)
.then(done, done.fail);
await expectAsync(callRule(rule0, empty(), context).toPromise()).toBeRejectedWithError(
InvalidRuleResultException,
);
});

it('works with undefined result', (done) => {
Expand Down

0 comments on commit 17eb20c

Please sign in to comment.