Skip to content

Commit

Permalink
fix(ngcc): prevent reflected decorators from being clobbered (#33362)
Browse files Browse the repository at this point in the history
ngcc has an internal cache of computed decorator information for
reflected classes, which could previously be mutated by consumers of the
reflection host. With the ability to inject synthesized decorators, such
decorators would inadvertently be added into the array of decorators
that was owned by the internal cache of the reflection host, incorrectly
resulting in synthesized decorators to be considered real decorators on
a class. This commit fixes the issue by cloning the cached array before
returning it.

PR Close #33362
  • Loading branch information
JoostK authored and AndrewKushnir committed Oct 25, 2019
1 parent dcdb433 commit 0de2dbf
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
12 changes: 10 additions & 2 deletions packages/compiler-cli/ngcc/src/host/esm2015_host.ts
Expand Up @@ -344,10 +344,18 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
return superDeclaration;
}

/** Gets all decorators of the given class symbol. */
/**
* Gets all decorators of the given class symbol. Any decorator that have been synthetically
* injected by a migration will not be present in the returned collection.
*/
getDecoratorsOfSymbol(symbol: NgccClassSymbol): Decorator[]|null {
const {classDecorators} = this.acquireDecoratorInfo(symbol);
return classDecorators;
if (classDecorators === null) {
return null;
}

// Return a clone of the array to prevent consumers from mutating the cache.
return Array.from(classDecorators);
}

/**
Expand Down
14 changes: 14 additions & 0 deletions packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts
Expand Up @@ -1931,6 +1931,20 @@ runInEachFileSystem(() => {
expect(classDecoratorsSecondary.length).toEqual(1);
expect(classDecoratorsSecondary[0] !.map(d => d.name)).toEqual(['Directive']);
});

it('should return a cloned array on each invocation', () => {
loadTestFiles(DECORATED_FILES);
const {program} = makeTestBundleProgram(getRootFiles(DECORATED_FILES)[0]);
const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker());
const classDecl =
getDeclaration(program, DECORATED_FILES[0].name, 'A', ts.isClassDeclaration) !;
const classSymbol = host.getClassSymbol(classDecl) !;

const firstResult = host.getDecoratorsOfSymbol(classSymbol);
const secondResult = host.getDecoratorsOfSymbol(classSymbol);

expect(firstResult).not.toBe(secondResult);
});
});

describe('getDtsDeclarationsOfClass()', () => {
Expand Down

0 comments on commit 0de2dbf

Please sign in to comment.