fix(core): do not use unbound attributes as inputs to structural directives (#36441)
Prior to this commit unbound attributes were treated as possible inputs to structural directives. Since structural directives can only accepts inputs defined using microsyntax expression (e.g. `<div *dir="exp">`), such unbound attributes should not be considered as inputs. This commit aligns Ivy and View Engine behavior and avoids using unbound attributes as inputs to structural directives. PR Close #36441
This commit is contained in:
		
							parent
							
								
									28995dba19
								
							
						
					
					
						commit
						acf6075ca9
					
				| @ -30,7 +30,7 @@ import {SanitizerFn} from '../interfaces/sanitization'; | |||||||
| import {isComponentDef, isComponentHost, isContentQueryHost, isLContainer, isRootView} from '../interfaces/type_checks'; | import {isComponentDef, isComponentHost, isContentQueryHost, isLContainer, isRootView} from '../interfaces/type_checks'; | ||||||
| import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, InitPhaseState, INJECTOR, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, T_HOST, TData, TVIEW, TView, TViewType} from '../interfaces/view'; | import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, InitPhaseState, INJECTOR, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, T_HOST, TData, TVIEW, TView, TViewType} from '../interfaces/view'; | ||||||
| import {assertNodeOfPossibleTypes} from '../node_assert'; | import {assertNodeOfPossibleTypes} from '../node_assert'; | ||||||
| import {isNodeMatchingSelectorList} from '../node_selector_matcher'; | import {isInlineTemplate, isNodeMatchingSelectorList} from '../node_selector_matcher'; | ||||||
| import {enterView, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getPreviousOrParentTNode, getSelectedIndex, getTView, leaveView, setBindingIndex, setBindingRootForHostBindings, setCheckNoChangesMode, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state'; | import {enterView, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getPreviousOrParentTNode, getSelectedIndex, getTView, leaveView, setBindingIndex, setBindingRootForHostBindings, setCheckNoChangesMode, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state'; | ||||||
| import {NO_CHANGE} from '../tokens'; | import {NO_CHANGE} from '../tokens'; | ||||||
| import {isAnimationProp, mergeHostAttrs} from '../util/attrs_utils'; | import {isAnimationProp, mergeHostAttrs} from '../util/attrs_utils'; | ||||||
| @ -900,8 +900,14 @@ function initializeInputAndOutputAliases(tView: TView, tNode: TNode): void { | |||||||
|   for (let i = start; i < end; i++) { |   for (let i = start; i < end; i++) { | ||||||
|     const directiveDef = defs[i] as DirectiveDef<any>; |     const directiveDef = defs[i] as DirectiveDef<any>; | ||||||
|     const directiveInputs = directiveDef.inputs; |     const directiveInputs = directiveDef.inputs; | ||||||
|     inputsFromAttrs.push( |     // Do not use unbound attributes as inputs to structural directives, since structural
 | ||||||
|         tNodeAttrs !== null ? generateInitialInputs(directiveInputs, tNodeAttrs) : null); |     // directive inputs can only be set using microsyntax (e.g. `<div *dir="exp">`).
 | ||||||
|  |     // TODO(FW-1930): microsyntax expressions may also contain unbound/static attributes, which
 | ||||||
|  |     // should be set for inline templates.
 | ||||||
|  |     const initialInputs = (tNodeAttrs !== null && !isInlineTemplate(tNode)) ? | ||||||
|  |         generateInitialInputs(directiveInputs, tNodeAttrs) : | ||||||
|  |         null; | ||||||
|  |     inputsFromAttrs.push(initialInputs); | ||||||
|     inputsStore = generatePropertyAliases(directiveInputs, i, inputsStore); |     inputsStore = generatePropertyAliases(directiveInputs, i, inputsStore); | ||||||
|     outputsStore = generatePropertyAliases(directiveDef.outputs, i, outputsStore); |     outputsStore = generatePropertyAliases(directiveDef.outputs, i, outputsStore); | ||||||
|   } |   } | ||||||
|  | |||||||
| @ -56,6 +56,15 @@ function isCssClassMatching( | |||||||
|   return false; |   return false; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | /** | ||||||
|  |  * Checks whether the `tNode` represents an inline template (e.g. `*ngFor`). | ||||||
|  |  * | ||||||
|  |  * @param tNode current TNode | ||||||
|  |  */ | ||||||
|  | export function isInlineTemplate(tNode: TNode): boolean { | ||||||
|  |   return tNode.type === TNodeType.Container && tNode.tagName !== NG_TEMPLATE_SELECTOR; | ||||||
|  | } | ||||||
|  | 
 | ||||||
| /** | /** | ||||||
|  * Function that checks whether a given tNode matches tag-based selector and has a valid type. |  * Function that checks whether a given tNode matches tag-based selector and has a valid type. | ||||||
|  * |  * | ||||||
| @ -134,11 +143,9 @@ export function isNodeMatchingSelector( | |||||||
|         continue; |         continue; | ||||||
|       } |       } | ||||||
| 
 | 
 | ||||||
|       const isInlineTemplate = |  | ||||||
|           tNode.type == TNodeType.Container && tNode.tagName !== NG_TEMPLATE_SELECTOR; |  | ||||||
|       const attrName = (mode & SelectorFlags.CLASS) ? 'class' : current; |       const attrName = (mode & SelectorFlags.CLASS) ? 'class' : current; | ||||||
|       const attrIndexInNode = |       const attrIndexInNode = | ||||||
|           findAttrIndexInNode(attrName, nodeAttrs, isInlineTemplate, isProjectionMode); |           findAttrIndexInNode(attrName, nodeAttrs, isInlineTemplate(tNode), isProjectionMode); | ||||||
| 
 | 
 | ||||||
|       if (attrIndexInNode === -1) { |       if (attrIndexInNode === -1) { | ||||||
|         if (isPositive(mode)) return false; |         if (isPositive(mode)) return false; | ||||||
|  | |||||||
| @ -420,6 +420,93 @@ describe('directives', () => { | |||||||
| 
 | 
 | ||||||
|          expect(dirInstance!.dir).toBe('Hello'); |          expect(dirInstance!.dir).toBe('Hello'); | ||||||
|        }); |        }); | ||||||
|  | 
 | ||||||
|  |     it('should not set structural directive inputs from static element attrs', () => { | ||||||
|  |       const dirInstances: StructuralDir[] = []; | ||||||
|  | 
 | ||||||
|  |       @Directive({selector: '[dir]'}) | ||||||
|  |       class StructuralDir { | ||||||
|  |         constructor() { | ||||||
|  |           dirInstances.push(this); | ||||||
|  |         } | ||||||
|  |         @Input() dirOf!: number[]; | ||||||
|  |         @Input() dirUnboundInput: any; | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       @Component({ | ||||||
|  |         template: ` | ||||||
|  |           <!-- Regular form of structural directive --> | ||||||
|  |           <div *dir="let item of items" dirUnboundInput>Some content</div> | ||||||
|  | 
 | ||||||
|  |           <!-- De-sugared version of the same structural directive --> | ||||||
|  |           <ng-template dir let-item [dirOf]="items" dirUnboundInput> | ||||||
|  |             <div>Some content</div> | ||||||
|  |           </ng-template> | ||||||
|  |         `,
 | ||||||
|  |       }) | ||||||
|  |       class App { | ||||||
|  |         items: number[] = [1, 2, 3]; | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       TestBed.configureTestingModule({ | ||||||
|  |         declarations: [App, StructuralDir], | ||||||
|  |       }); | ||||||
|  |       const fixture = TestBed.createComponent(App); | ||||||
|  |       fixture.detectChanges(); | ||||||
|  | 
 | ||||||
|  |       const [regularDir, desugaredDir] = dirInstances; | ||||||
|  | 
 | ||||||
|  |       // When directive is used as a structural one, the `dirUnboundInput` should not be treated as
 | ||||||
|  |       // an input.
 | ||||||
|  |       expect(regularDir.dirUnboundInput).toBe(undefined); | ||||||
|  | 
 | ||||||
|  |       // In de-sugared version the `dirUnboundInput` acts as a regular input, so it should be set
 | ||||||
|  |       // to an empty string.
 | ||||||
|  |       expect(desugaredDir.dirUnboundInput).toBe(''); | ||||||
|  |     }); | ||||||
|  | 
 | ||||||
|  |     it('should not set structural directive inputs from element bindings', () => { | ||||||
|  |       const dirInstances: StructuralDir[] = []; | ||||||
|  | 
 | ||||||
|  |       @Directive({selector: '[dir]'}) | ||||||
|  |       class StructuralDir { | ||||||
|  |         constructor() { | ||||||
|  |           dirInstances.push(this); | ||||||
|  |         } | ||||||
|  |         @Input() dirOf!: number[]; | ||||||
|  |         @Input() title: any; | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       @Component({ | ||||||
|  |         template: ` | ||||||
|  |           <!-- Regular form of structural directive --> | ||||||
|  |           <div *dir="let item of items" [title]="title">Some content</div> | ||||||
|  | 
 | ||||||
|  |           <!-- De-sugared version of the same structural directive --> | ||||||
|  |           <ng-template dir let-item [dirOf]="items" [title]="title"> | ||||||
|  |             <div>Some content</div> | ||||||
|  |           </ng-template> | ||||||
|  |         `,
 | ||||||
|  |       }) | ||||||
|  |       class App { | ||||||
|  |         items: number[] = [1, 2, 3]; | ||||||
|  |         title: string = 'element title'; | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       TestBed.configureTestingModule({ | ||||||
|  |         declarations: [App, StructuralDir], | ||||||
|  |       }); | ||||||
|  |       const fixture = TestBed.createComponent(App); | ||||||
|  |       fixture.detectChanges(); | ||||||
|  | 
 | ||||||
|  |       const [regularDir, desugaredDir] = dirInstances; | ||||||
|  | 
 | ||||||
|  |       // When directive is used as a structural one, the `title` should not be treated as an input.
 | ||||||
|  |       expect(regularDir.title).toBe(undefined); | ||||||
|  | 
 | ||||||
|  |       // In de-sugared version the `title` acts as a regular input, so it should be set.
 | ||||||
|  |       expect(desugaredDir.title).toBe('element title'); | ||||||
|  |     }); | ||||||
|   }); |   }); | ||||||
| 
 | 
 | ||||||
|   describe('outputs', () => { |   describe('outputs', () => { | ||||||
|  | |||||||
| @ -461,6 +461,9 @@ | |||||||
|   { |   { | ||||||
|     "name": "isFactory" |     "name": "isFactory" | ||||||
|   }, |   }, | ||||||
|  |   { | ||||||
|  |     "name": "isInlineTemplate" | ||||||
|  |   }, | ||||||
|   { |   { | ||||||
|     "name": "isLContainer" |     "name": "isLContainer" | ||||||
|   }, |   }, | ||||||
|  | |||||||
| @ -860,6 +860,9 @@ | |||||||
|   { |   { | ||||||
|     "name": "isInHostBindings" |     "name": "isInHostBindings" | ||||||
|   }, |   }, | ||||||
|  |   { | ||||||
|  |     "name": "isInlineTemplate" | ||||||
|  |   }, | ||||||
|   { |   { | ||||||
|     "name": "isJsObject" |     "name": "isJsObject" | ||||||
|   }, |   }, | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user