https://github.com/angular/angular
Revision e6ee6d25f93e031db02e2ef38ab2650f0ba49936 authored by JoostK on 10 March 2024, 13:39:32 UTC, committed by Andrew Scott on 12 March 2024, 21:05:16 UTC
This commit resolves a regression that was introduced when the compiler switched from
`TemplateDefinitionBuilder` (TDB) to the template pipeline (TP) compiler. The TP compiler
has changed the output of

```html
if (false) { <div class="test"></div> }
```

from

```ts
defineComponent({
  consts: [['class', 'test'], [AttributeMarker.Classes, 'test']],
  template: function(rf) {
    if (rf & 1) {
      ɵɵtemplate(0, App_Conditional_0_Template, 2, 0, "div", 0)
    }
  }
});
```

to

```ts
defineComponent({
  consts: [[AttributeMarker.Classes, 'test']],
  template: function(rf) {
    if (rf & 1) {
      ɵɵtemplate(0, App_Conditional_0_Template, 2, 0, "div", 0)
    }
  }
});
```

The last argument to the `ɵɵtemplate` instruction (0 in both compilation outputs) corresponds with
the index in `consts` of the element's attribute's, and we observe how TP has allocated only a single
attribute array for the `div`, where there used to be two `consts` entries with TDB. Consequently,
the `ɵɵtemplate` instruction is now effectively referencing a different attributes array, where the
distinction between the `"class"` attribute vs. the `AttributeMarker.Classes` distinction affects
the behavior: TP's emit causes the runtime to incorrectly match a directive with `selector: '.foo'` to
be instantiated on the `ɵɵtemplate` instruction as if it corresponds with a structural directive!

Instead of changing TP to align with TDB's emit, this commit updates the runtime instead. This uncovered
an inconsistency in selector matching for class names, where there used to be two paths dealing with
class matching:

1. The first check was commented to be a special-case for class matching, implemented in `isCssClassMatching`.
2. The second path was part of the main selector matching algorithm, where `findAttrIndexInNode` was being used
   to find the start position in `tNode.attrs` to match the selector's value against.

The second path only considers `AttributeMarker.Classes` values if matching for content projection, OR of the
`TNode` is not an inline template. The special-case in path 1 however does not make that distinction, so it
would consider the `AttributeMarker.Classes` binding as a selector match, incorrectly causing a directive to
match on the `ɵɵtemplate` itself.

The second path was also buggy for class bindings, as the return value of `classIndexOf` was incorrectly
negated: it considered a matching class attribute as non-matching and vice-versa. This bug was not observable
because of another issue, where the class-handling in part 2 was never relevant because of the special-case
in part 1.

This commit separates path 1 entirely from path 2 and removes the buggy class-matching logic in part 2, as
that is entirely handled by path 1 anyway. `isCssClassMatching` is updated to exclude class bindings from
being matched for inline templates.

Fixes #54798

PR Close #54800
1 parent 0f7bbd0
History
Tip revision: e6ee6d25f93e031db02e2ef38ab2650f0ba49936 authored by JoostK on 10 March 2024, 13:39:32 UTC
fix(core): exclude class attribute intended for projection matching from directive matching (#54800)
Tip revision: e6ee6d2
File Mode Size
.circleci
.devcontainer
.github
.husky
.ng-dev
.vscode
.yarn
adev
aio
devtools
docs
goldens
integration
modules
packages
scripts
third_party
tools
.bazelignore -rw-r--r-- 1.6 KB
.bazelrc -rw-r--r-- 7.8 KB
.bazelversion -rw-r--r-- 6 bytes
.clang-format -rw-r--r-- 73 bytes
.editorconfig -rw-r--r-- 245 bytes
.gitattributes -rw-r--r-- 146 bytes
.gitignore -rw-r--r-- 1004 bytes
.gitmessage -rw-r--r-- 7.2 KB
.mailmap -rw-r--r-- 51 bytes
.npmrc -rw-r--r-- 21 bytes
.nvmrc -rw-r--r-- 8 bytes
.prettierrc -rw-r--r-- 196 bytes
.pullapprove.yml -rw-r--r-- 51.8 KB
.yarnrc -rw-r--r-- 130 bytes
BUILD.bazel -rw-r--r-- 2.5 KB
CHANGELOG.md -rw-r--r-- 282.7 KB
CHANGELOG_ARCHIVE.md -rw-r--r-- 997.0 KB
CODE_OF_CONDUCT.md -rw-r--r-- 3.6 KB
CONTRIBUTING.md -rw-r--r-- 15.7 KB
LICENSE -rw-r--r-- 1.1 KB
README.md -rw-r--r-- 5.5 KB
SECURITY.md -rw-r--r-- 403 bytes
WORKSPACE -rw-r--r-- 10.1 KB
browser-providers.conf.d.ts -rw-r--r-- 387 bytes
browser-providers.conf.js -rw-r--r-- 1.9 KB
gulpfile.js -rw-r--r-- 572 bytes
karma-js.conf.js -rw-r--r-- 6.0 KB
package.json -rw-r--r-- 9.9 KB
packages.bzl -rw-r--r-- 3.8 KB
renovate.json -rw-r--r-- 3.0 KB
tsconfig-tslint.json -rw-r--r-- 171 bytes
tslint.json -rw-r--r-- 5.3 KB
yarn.bzl -rw-r--r-- 77 bytes
yarn.lock -rw-r--r-- 805.1 KB
yarn.lock.readme.md -rw-r--r-- 1.3 KB

README.md

back to top