fix(compiler): Metadata should not include methods on Object.prototype (#38292)
This commit fixes a bug in View Engine whereby the compiler errorneously
thinks that a method of a component has decorator metadata when that
method is one of those in `Object.prototype`, for example `toString`.
This bug is discovered in v10.0.4 of `@angular/language-service` after
the default bundle format was switched from ES5 to ES2015.
ES5 output:
```js
if (propMetadata[propName]) {
    decorators.push.apply(decorators, __spread(propMetadata[propName]));
}
```
ES2015 output:
```js
if (propMetadata[propName]) {
    decorators.push(...propMetadata[propName]);
}
```
The bug was not discovered in ES5 because the polyfill for the spread
operator happily accepts parameters that do not have the `iterable`
symbol:
```js
function __spread() {
    for (var ar = [], i = 0; i < arguments.length; i++)
        ar = ar.concat(__read(arguments[i]));
    return ar;
}
```
whereas in es2015 it’ll fail since the iterable symbol is not present in
`propMetadata['toString']` which evaluates to a function.
Fixes https://github.com/angular/vscode-ng-language-service/issues/859
PR Close #38292
			
			
This commit is contained in:
		
							parent
							
								
									3a525d196b
								
							
						
					
					
						commit
						fd51e01335
					
				| @ -235,7 +235,9 @@ export class StaticReflector implements CompileReflector { | |||||||
|         const prop = (<any[]>propData) |         const prop = (<any[]>propData) | ||||||
|                          .find(a => a['__symbolic'] == 'property' || a['__symbolic'] == 'method'); |                          .find(a => a['__symbolic'] == 'property' || a['__symbolic'] == 'method'); | ||||||
|         const decorators: any[] = []; |         const decorators: any[] = []; | ||||||
|         if (propMetadata![propName]) { |         // hasOwnProperty() is used here to make sure we do not look up methods
 | ||||||
|  |         // on `Object.prototype`.
 | ||||||
|  |         if (propMetadata?.hasOwnProperty(propName)) { | ||||||
|           decorators.push(...propMetadata![propName]); |           decorators.push(...propMetadata![propName]); | ||||||
|         } |         } | ||||||
|         propMetadata![propName] = decorators; |         propMetadata![propName] = decorators; | ||||||
|  | |||||||
| @ -777,6 +777,35 @@ describe('StaticReflector', () => { | |||||||
|           .toEqual({}); |           .toEqual({}); | ||||||
|     }); |     }); | ||||||
| 
 | 
 | ||||||
|  |     it('should not inherit methods from Object.prototype', () => { | ||||||
|  |       const filePath = '/tmp/test.ts'; | ||||||
|  |       init({ | ||||||
|  |         ...DEFAULT_TEST_DATA, | ||||||
|  |         [filePath]: ` | ||||||
|  |           import {Component} from '@angular/core'; | ||||||
|  | 
 | ||||||
|  |           @Component({ | ||||||
|  |             selector: 'test-component', | ||||||
|  |           }) | ||||||
|  |           export class TestComponent { | ||||||
|  |             title = 'Hello World'; | ||||||
|  | 
 | ||||||
|  |             toString() { | ||||||
|  |               return 'Test Component'; | ||||||
|  |             } | ||||||
|  |           } | ||||||
|  |         `,
 | ||||||
|  |       }); | ||||||
|  |       const declaration = reflector.getStaticSymbol(filePath, 'TestComponent'); | ||||||
|  |       expect(declaration.filePath).toBe(filePath); | ||||||
|  |       expect(declaration.name).toBe('TestComponent'); | ||||||
|  |       const propMetadata = reflector.propMetadata(declaration); | ||||||
|  |       // 'toString' is a member of TestComponent so it should be part of the metadata.
 | ||||||
|  |       expect(propMetadata.hasOwnProperty('toString')).toBe(true); | ||||||
|  |       // There are no decorators on 'toString' so it should be an empty array.
 | ||||||
|  |       expect(propMetadata['toString']).toEqual([]); | ||||||
|  |     }); | ||||||
|  | 
 | ||||||
|     it('should inherit lifecycle hooks', () => { |     it('should inherit lifecycle hooks', () => { | ||||||
|       initWithDecorator({ |       initWithDecorator({ | ||||||
|         '/tmp/src/main.ts': ` |         '/tmp/src/main.ts': ` | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user