fix(compiler-cli): use `Map` rather than `object` for map of partial linkers (#40563)

Previously, we were naïvely checking whether a function name was a partial linker
declaration call by testing the map of linkers with `linkers[name]`. Since
`linkers` was a plain object, it also matched function names like `toString`!

This has been refactored as a `Map` to avoid the problem.

PR Close #40563
This commit is contained in:
Pete Bacon Darwin 2021-01-25 21:26:06 +00:00 committed by Jessica Janiuk
parent 89d8caef93
commit b630b09c7e
2 changed files with 26 additions and 15 deletions

View File

@ -19,6 +19,11 @@ export const ɵɵngDeclareDirective = 'ɵɵngDeclareDirective';
export const ɵɵngDeclareComponent = 'ɵɵngDeclareComponent'; export const ɵɵngDeclareComponent = 'ɵɵngDeclareComponent';
export const declarationFunctions = [ɵɵngDeclareDirective, ɵɵngDeclareComponent]; export const declarationFunctions = [ɵɵngDeclareDirective, ɵɵngDeclareComponent];
interface LinkerRange<TExpression> {
range: string;
linker: PartialLinker<TExpression>;
}
/** /**
* A helper that selects the appropriate `PartialLinker` for a given declaration. * A helper that selects the appropriate `PartialLinker` for a given declaration.
* *
@ -35,7 +40,7 @@ export const declarationFunctions = [ɵɵngDeclareDirective, ɵɵngDeclareCompon
* allows the linker to work on local builds effectively. * allows the linker to work on local builds effectively.
*/ */
export class PartialLinkerSelector<TStatement, TExpression> { export class PartialLinkerSelector<TStatement, TExpression> {
private readonly linkers: Record<string, {range: string, linker: PartialLinker<TExpression>}[]>; private readonly linkers: Map<string, LinkerRange<TExpression>[]>;
constructor( constructor(
environment: LinkerEnvironment<TStatement, TExpression>, sourceUrl: AbsoluteFsPath, environment: LinkerEnvironment<TStatement, TExpression>, sourceUrl: AbsoluteFsPath,
@ -47,7 +52,7 @@ export class PartialLinkerSelector<TStatement, TExpression> {
* Returns true if there are `PartialLinker` classes that can handle functions with this name. * Returns true if there are `PartialLinker` classes that can handle functions with this name.
*/ */
supportsDeclaration(functionName: string): boolean { supportsDeclaration(functionName: string): boolean {
return this.linkers[functionName] !== undefined; return this.linkers.has(functionName);
} }
/** /**
@ -55,10 +60,10 @@ export class PartialLinkerSelector<TStatement, TExpression> {
* Throws an error if there is none. * Throws an error if there is none.
*/ */
getLinker(functionName: string, version: string): PartialLinker<TExpression> { getLinker(functionName: string, version: string): PartialLinker<TExpression> {
const versions = this.linkers[functionName]; if (!this.linkers.has(functionName)) {
if (versions === undefined) {
throw new Error(`Unknown partial declaration function ${functionName}.`); throw new Error(`Unknown partial declaration function ${functionName}.`);
} }
const versions = this.linkers.get(functionName)!;
for (const {range, linker} of versions) { for (const {range, linker} of versions) {
if (satisfies(version, range, {includePrerelease: true})) { if (satisfies(version, range, {includePrerelease: true})) {
return linker; return linker;
@ -71,21 +76,21 @@ export class PartialLinkerSelector<TStatement, TExpression> {
private createLinkerMap( private createLinkerMap(
environment: LinkerEnvironment<TStatement, TExpression>, sourceUrl: AbsoluteFsPath, environment: LinkerEnvironment<TStatement, TExpression>, sourceUrl: AbsoluteFsPath,
code: string) { code: string): Map<string, LinkerRange<TExpression>[]> {
const partialDirectiveLinkerVersion1 = new PartialDirectiveLinkerVersion1(sourceUrl, code); const partialDirectiveLinkerVersion1 = new PartialDirectiveLinkerVersion1(sourceUrl, code);
const partialComponentLinkerVersion1 = new PartialComponentLinkerVersion1( const partialComponentLinkerVersion1 = new PartialComponentLinkerVersion1(
environment, createGetSourceFile(sourceUrl, code, environment.sourceFileLoader), sourceUrl, environment, createGetSourceFile(sourceUrl, code, environment.sourceFileLoader), sourceUrl,
code); code);
return { const linkers = new Map<string, LinkerRange<TExpression>[]>();
[ɵɵngDeclareDirective]: [ linkers.set(ɵɵngDeclareDirective, [
{range: '0.0.0-PLACEHOLDER', linker: partialDirectiveLinkerVersion1}, {range: '0.0.0-PLACEHOLDER', linker: partialDirectiveLinkerVersion1},
{range: '>=11.1.0-next.1', linker: partialDirectiveLinkerVersion1}, {range: '>=11.1.0-next.1', linker: partialDirectiveLinkerVersion1},
], ]);
[ɵɵngDeclareComponent]: [ linkers.set(ɵɵngDeclareComponent, [
{range: '0.0.0-PLACEHOLDER', linker: partialComponentLinkerVersion1}, {range: '0.0.0-PLACEHOLDER', linker: partialComponentLinkerVersion1},
{range: '>=11.1.0-next.1', linker: partialComponentLinkerVersion1}, {range: '>=11.1.0-next.1', linker: partialComponentLinkerVersion1},
], ]);
}; return linkers;
} }
} }

View File

@ -44,6 +44,12 @@ describe('PartialLinkerSelector', () => {
expect(selector.supportsDeclaration('ɵɵngDeclareComponent')).toBe(true); expect(selector.supportsDeclaration('ɵɵngDeclareComponent')).toBe(true);
expect(selector.supportsDeclaration('$foo')).toBe(false); expect(selector.supportsDeclaration('$foo')).toBe(false);
}); });
it('should return false for methods on `Object`', () => {
const selector = new PartialLinkerSelector(
environment, fs.resolve('/some/path/to/file.js'), 'some file contents');
expect(selector.supportsDeclaration('toString')).toBe(false);
});
}); });
describe('getLinker()', () => { describe('getLinker()', () => {