GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — refactor(compiler): disallow unsupported bindings and attributes on ng-content and ng-template by JeanMeche · Pull Request #71887 · angular/angular Conversation
|
| const attributes = new Set<TextAttribute>(); | ||
| for (const attr of child.attributes) { | ||
| const withoutAttr = removeAttributeFromSelector(selector, attr.name); | ||
| if (findProjectionSlot(matcher, ngContentSelectors, withoutAttr) !== slot) { |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Two subtle edge cases with checking each attribute by removing it individually (withoutAttr = removeAttributeFromSelector(selector, attr.name)):
class="...": Because removeAttributeFromSelector strips the entire class attribute and all selector.classNames at once (lines 271–275), if a parent has <ng-content select=".used" /> and a child writes <ng-content class="used unused" />, removing class changes the matched slot so the whole TextAttribute is marked as used—allowing the redundant unused class token through without a diagnostic.<ng-content select="[slot], .foo" /> and a child writes <ng-content slot class="foo" />, removing slot alone still matches slot 0 via .foo, and removing class alone still matches slot 0 via [slot]. Because neither attribute changes slot when removed on its own, both attributes will be flagged as unsupported even though at least one is required to match the slot.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| function createContentCssSelector(node: Content): CssSelector { | ||
| const selector = new CssSelector(); | ||
| for (const attr of node.attributes) { | ||
| const name = splitNsName(attr.name)[1]; |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
splitNsName(attr.name)[1] strips the namespace prefix and adds the unprefixed name to CssSelector. However, at runtime Angular's projection matcher (findAttrIndexInNode in packages/core/src/render3/node_selector_matcher.ts) explicitly skips AttributeMarker.NamespaceURI attributes altogether. If <ng-content> has a namespaced attribute (e.g. xlink:title), createContentCssSelector will match a parent's [title] slot even though it won't match at runtime. Consider skipping attributes where splitNsName(attr.name)[0] !== null.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| if ( | ||
| meta === undefined || | ||
| meta.ngContentSelectors === null || | ||
| meta.ngContentSelectors.length === 0 |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
In ngtsc's DtsMetadataReader (packages/compiler-cli/src/ngtsc/metadata/src/dts.ts), a .d.ts component with no <ng-content> slots (NgContentSelectors = never) has ngContentSelectors: [], whereas ngContentSelectors: null indicates a legacy .d.ts declaration (typeArguments.length <= 6) where projection slots were not recorded in the type definition.
Returning early here when meta.ngContentSelectors === null treats unknown slots the same as zero slots, which could produce false-positive diagnostics if <ng-content someAttr> is projected into a component from an older .d.ts.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| references: t.Reference[], | ||
| attributes: t.TextAttribute[], | ||
| ): void { | ||
| if (boundEvents.length > 0) { |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Nit: boundEvents, parsedProperties, and references only report the first item ([0]), whereas attributes below (and validateNoAttributeBindings) loop over every item. Iterating over all elements in boundEvents, parsedProperties, and references would report all unsupported bindings on <ng-content> in one pass rather than requiring the user to fix them one at a time.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
797374e to
f6014b5
Compare
…g-content and ng-template
Virtual elements such as <ng-content> and <ng-template> do not support
certain bindings and attributes:
- <ng-template> does not support attribute bindings ([attr.foo]).
- <ng-content> does not support property bindings ([id]), event bindings ((click)),
or template references (#ref).
- <ng-content> only supports an allowlist of static attributes ('select',
'ngProjectAs', 'ngSkipHydration', 'ngNonBindable', 'ngPreserveWhitespaces',
and i18n attributes 'i18n', 'i18n-*'), as well as static attributes that
select a content projection slot of a parent component when re-projecting
<ng-content>. If 'selection' or 'selector' is encountered on <ng-content>,
a helpful 'Did you mean select?' suggestion is provided.
Syntactic binding and reference checks are enforced during template
transformation (r3_template_transform.ts), while static attribute checks on
<ng-content> are performed during TCB generation (TcbContentAttributesOp) so
that parent component projection slots can be taken into account across both
ngtsc and ngp.
Also removes legacy compliance tests in r3_view_compiler_template that asserted
unsupported attribute and class bindings on ng-template degraded to property
bindings, and fixes tests and examples where invalid attributes were used on
<ng-content>.
f6014b5 to
41017f3
Compare
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.