fix(compiler-cli): Handle ng-template with structural directive in indexer by atscott · Pull Request #44788 · angular/angular

atscott · 2022-01-22T00:38:10Z

An ng-template with an inline template (i.e. has a structural
directive) would previously not get an undefined tagName because the
logic assumed the element would be t.Element or t.Content and read
the tag name from the name property. For a t.Template, this exists
instead on the t.tagName. The final result would be an tagName of undefined
for the parent t.Template, causing failures in the indexer downstream.

This undefined value is actually expected in the renderer code, even
though the type does not specify this possibility. This change updates
the type of tagName to be string|null and explicitly handles the
case where there is a structural directive on an ng-template.

atscott · 2022-01-22T01:12:21Z

~~Note to self: This requires more investigation. The tests fail because of this code:~~

angular/packages/compiler/src/render3/view/template.ts

Lines 872 to 883 in 80467c3

    
           const tagNameWithoutNamespace = 
        
               template.tagName ? splitNsName(template.tagName)[1] : template.tagName; 
        
           const contextName = `${this.contextName}${ 
        
               template.tagName ? '_' + sanitizeIdentifier(template.tagName) : ''}_${templateIndex}`; 
        
           const templateName = `${contextName}_Template`; 
        
           const parameters: o.Expression[] = [ 
        
             o.literal(templateIndex), 
        
             o.variable(templateName), 
        
             // We don't care about the tag's namespace here, because we infer 
        
             // it based on the parent nodes inside the template instruction. 
        
             o.literal(tagNameWithoutNamespace), 
        
           ];

~~The synthetic ng-template created by the structural directive and the actual ng-template now have the same names (rather than the synthetic one having undefined)~~

Edit: This has been addressed.

atscott · 2022-01-24T18:12:27Z

presubmit

…indexer An `ng-template` with an inline template (i.e. has a structural directive) would previously not get an `undefined` `tagName` because the logic assumed the element would be `t.Element` or `t.Content` and read the tag name from the `name` property. For a `t.Template`, this exists instead on the `t.tagName`. The final result would be an `tagName` of `undefined` for the parent `t.Template`, causing failures in the indexer downstream. This `undefined` value is actually expected in the renderer code, even though the type does not specify this possibility. This change updates the type of `tagName` to be `string|null` and explicitly handles the case where there is a structural directive on an `ng-template`. You can see how the two are differentiated in the compliance code that was modified in this commit.

AndrewKushnir

👍

AndrewKushnir · 2022-01-25T22:15:47Z

This PR was merged into the repository by commit 1bce51c.

…indexer (#44788) An `ng-template` with an inline template (i.e. has a structural directive) would previously not get an `undefined` `tagName` because the logic assumed the element would be `t.Element` or `t.Content` and read the tag name from the `name` property. For a `t.Template`, this exists instead on the `t.tagName`. The final result would be an `tagName` of `undefined` for the parent `t.Template`, causing failures in the indexer downstream. This `undefined` value is actually expected in the renderer code, even though the type does not specify this possibility. This change updates the type of `tagName` to be `string|null` and explicitly handles the case where there is a structural directive on an `ng-template`. You can see how the two are differentiated in the compliance code that was modified in this commit. PR Close #44788

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fanimations/13.1.2/13.2.1) | | [@angular/common](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcommon/13.1.2/13.2.1) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/13.1.2/13.2.1) | | [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/13.1.2/13.2.1) | | [@angular/core](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fcore/13.1.2/13.2.1) | | [@angular/forms](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fforms/13.1.2/13.2.1) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/13.1.2/13.2.1) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/13.1.2/13.2.1) | | [@angular/router](https://github.com/angular/angular) | dependencies | minor | [`13.1.2` -> `13.2.1`](https://renovatebot.com/diffs/npm/@angular%2frouter/13.1.2/13.2.1) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v13.2.1`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1321-2022-02-02) [Compare Source](angular/angular@13.2.0...13.2.1) ##### animations | Commit | Type | Description | | -- | -- | -- | | [4644886aaf](angular/angular@4644886) | perf | remove no longer needed CssKeyframes classes ([#44903](angular/angular#44903)) ([#44919](angular/angular#44919)) | ##### common | Commit | Type | Description | | -- | -- | -- | | [b4e4617807](angular/angular@b4e4617) | fix | include query parameters for open HTTP requests in `verify` ([#44917](angular/angular#44917)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [0778e6f7d7](angular/angular@0778e6f) | fix | accept nullish coalescing operator for any and unknown types ([#44862](angular/angular#44862)) | | [07185f4ed1](angular/angular@07185f4) | fix | enable nullish coalescing check only with `strictNullChecks` ([#44862](angular/angular#44862)) | | [4a5ad1793f](angular/angular@4a5ad17) | fix | ensure casing of logical paths is preserved ([#44798](angular/angular#44798)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [7ec482d9c2](angular/angular@7ec482d) | fix | Add back support for namespace URIs in createElement of dom renderer ([#44914](angular/angular#44914)) | | [250dc40a46](angular/angular@250dc40) | fix | flush delayed scoping queue while setting up TestBed ([#44814](angular/angular#44814)) | ##### forms | Commit | Type | Description | | -- | -- | -- | | [1aebbf8714](angular/angular@1aebbf8) | fix | ensure OnPush ancestors are marked dirty when the promise resolves ([#44886](angular/angular#44886)) | | [6b7fffcbeb](angular/angular@6b7fffc) | fix | Update the typed forms migration schematic to find all files. ([#44881](angular/angular#44881)) | #### Special Thanks Alan, Andrew Kushnir, Andrew Scott, Aristeidis Bampakos, Arjen, Daniel Díaz, David Shevitz, Doug Parker, Dylan Hunn, Esteban Gehring, George Kalpakas, Jessica Janiuk, JoostK, Juri Strumpflohner, Lee Robinson, Maarten Tibau, Paul Gschwendtner, Theodore Brown, arturovt, dario-piotrowicz, fru2, markostanimirovic and mgechev  ### [`v13.2.0`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1320-2022-01-26) [Compare Source](angular/angular@13.1.3...13.2.0) #### Deprecations ##### - The `CachedResourceLoader` and `RESOURCE_CACHE_PROVIDER` symbols were previously necessary in some cases to test AOT-compiled components with View Engine, but they are no longer needed since Ivy. - The `ComponentFactory` and `ComponentFactoryResolver` classes are deprecated. Since Ivy, there is no need to resolve Component factories. Please use other APIs where you Component classes can be used directly (without resolving their factories). - Since Ivy, the `CompilerOptions.useJit` and `CompilerOptions.missingTranslation` config options are unused, passing them has no effect. ##### | Commit | Type | Description | | -- | -- | -- | | [9c11183e74](angular/angular@9c11183) | docs | deprecate `CachedResourceLoader` and `RESOURCE_CACHE_PROVIDER` symbols ([#44749](angular/angular#44749)) | | [9f12e7fea4](angular/angular@9f12e7f) | docs | deprecate `ComponentFactory` and `ComponentFactoryResolver` symbols ([#44749](angular/angular#44749)) | | [4e95a316ce](angular/angular@4e95a31) | docs | deprecate unused config options from the `CompilerOptions` interface ([#44749](angular/angular#44749)) | ##### compiler | Commit | Type | Description | | -- | -- | -- | | [a4ab6d6b72](angular/angular@a4ab6d6) | feat | add support for safe calls in templates ([#44580](angular/angular#44580)) | | [abd1bc8039](angular/angular@abd1bc8) | fix | correct spans when parsing bindings with comments ([#44785](angular/angular#44785)) | | [ed67a074ce](angular/angular@ed67a07) | fix | properly compile DI factories when coverage reporting is enabled ([#44732](angular/angular#44732)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [fa835b5a29](angular/angular@fa835b5) | feat | enable extended diagnostics by default ([#44712](angular/angular#44712)) | | [73424def13](angular/angular@73424de) | feat | provide the animations for `DirectiveMeta` ([#44630](angular/angular#44630)) | | [fe3e4d6865](angular/angular@fe3e4d6) | fix | Handle `ng-template` with structural directive in indexer ([#44788](angular/angular#44788)) | | [7316e72ec5](angular/angular@7316e72) | fix | properly index <svg> elements when on a template ([#44785](angular/angular#44785)) | | [100091ebf0](angular/angular@100091e) | fix | remove leftover `_extendedTemplateDiagnostics` requirements ([#44777](angular/angular#44777)) | | [d2ae96f742](angular/angular@d2ae96f) | fix | skip `ExtendedTemplateCheckerImpl` construction if there were configuration errors ([#44778](angular/angular#44778)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [5626b34264](angular/angular@5626b34) | fix | consistently use namespace short name rather than URI ([#44766](angular/angular#44766)) | | [94bfcdd9de](angular/angular@94bfcdd) | fix | error if NgZone.isInAngularZone is called with a noop zone ([#44800](angular/angular#44800)) | ##### forms | Commit | Type | Description | | -- | -- | -- | | [72092ebd26](angular/angular@72092eb) | feat | Allow a FormControl to use initial value as default. ([#44434](angular/angular#44434)) | | [f7aa937cac](angular/angular@f7aa937) | fix | Make some minor fixups for forward-compatibility with typed forms. ([#44540](angular/angular#44540)) | ##### router | Commit | Type | Description | | -- | -- | -- | | [5a4ddfd4f5](angular/angular@5a4ddfd) | feat | Allow symbol keys for `Route` `data` and `resolve` properties ([#44519](angular/angular#44519)) | #### Special Thanks Alex Rickabaugh, Andrew Kushnir, Andrew Scott, Dario Piotrowicz, Derek Cormier, Doug Parker, Douglas Parker, Dylan Hunn, George Kalpakas, Jessica Janiuk, JoostK, Kristiyan Kostadinov, Martin Probst, Oleg Postoev, Stephanie Tuerk, Tim Bowersox, Wiley Marques, Yousaf Nawaz, dario-piotrowicz, iRealNirmal, ivanwonder and shejialuo  ### [`v13.1.3`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1313-2022-01-19) [Compare Source](angular/angular@13.1.2...13.1.3) ##### animations | Commit | Type | Description | | -- | -- | -- | | [af0a152a2c](angular/angular@af0a152) | fix | apply setStyles to only rootTimelines ([#44515](angular/angular#44515)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [626f3f230b](angular/angular@626f3f2) | perf | reduce analysis work during incremental rebuilds ([#44731](angular/angular#44731)) | ##### ngcc | Commit | Type | Description | | -- | -- | -- | | [f9ca4d8499](angular/angular@f9ca4d8) | fix | support element accesses for export declarations ([#44669](angular/angular#44669)) | #### Special Thanks Alan Agius, Andrew Kushnir, AnkitSharma-007, Daniel Díaz, Dmytro Mezhenskyi, Jessica Janiuk, Joey Perrott, JoostK, Ramesh Thiruchelvam, dario-piotrowicz, iRealNirmal and Łukasz Holeczek  </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Co-authored-by: crapStone <crapstone@noreply.codeberg.org> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1148 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

angular-automatic-lock-bot · 2022-02-25T16:07:42Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

atscott force-pushed the ngtemplateindexerbug branch 4 times, most recently from 0a3ef57 to fcdb207 Compare January 24, 2022 17:56

atscott marked this pull request as ready for review January 24, 2022 18:00

atscott force-pushed the ngtemplateindexerbug branch from fcdb207 to d36362d Compare January 24, 2022 18:11

atscott force-pushed the ngtemplateindexerbug branch 2 times, most recently from 6572eed to 6a1da54 Compare January 24, 2022 19:20

atscott changed the title ~~fix(compiler): Use correct name for ng-template with structural directive~~ fix(compiler-cli): Handle ng-template with structural directive for indexer Jan 25, 2022

atscott changed the title ~~fix(compiler-cli): Handle ng-template with structural directive for indexer~~ fix(compiler-cli): Handle ng-template with structural directive in indexer Jan 25, 2022

atscott force-pushed the ngtemplateindexerbug branch from 6a1da54 to 40655e6 Compare January 25, 2022 00:52

AndrewKushnir approved these changes Jan 25, 2022

View reviewed changes

atscott added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Jan 25, 2022

AndrewKushnir closed this in 1bce51c Jan 25, 2022

angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(compiler-cli): Handle ng-template with structural directive in indexer#44788

fix(compiler-cli): Handle ng-template with structural directive in indexer#44788
atscott wants to merge 1 commit intoangular:masterfrom
atscott:ngtemplateindexerbug

atscott commented Jan 22, 2022 •

edited

Loading

Uh oh!

atscott commented Jan 22, 2022 •

edited

Loading

Uh oh!

atscott commented Jan 24, 2022

Uh oh!

AndrewKushnir left a comment

Uh oh!

AndrewKushnir commented Jan 25, 2022

Uh oh!

angular-automatic-lock-bot bot commented Feb 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

atscott commented Jan 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atscott commented Jan 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atscott commented Jan 24, 2022

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented Jan 25, 2022

Uh oh!

angular-automatic-lock-bot bot commented Feb 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

atscott commented Jan 22, 2022 •

edited

Loading

atscott commented Jan 22, 2022 •

edited

Loading