fix($compile): support transcluding multi-element directives #15555
Conversation
| @@ -2480,7 +2480,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { | |||
|
|
|||
| // We have transclusion slots, | |||
| // collect them up, compile them and store their transclusion functions | |||
| $template = []; | |||
| $template = window.document.createDocumentFragment(); | |||
There was a problem hiding this comment.
Should this use $window or $document?
There was a problem hiding this comment.
Other places inside this file use window directly (e.g. here, here and here). Normally, I would prefer $window (or $document). But it doesn't bother me too much here because:
- This file is dealing with pretty "low-level" stuff.
- Switching to
$windowwould be a breaking change (since every$windowor$documentmock should then provide the necessary APIs).
|
@gkalpak what's the status of this? Seems like a reasonable fix... |
|
Status: Waiting for approval 😁 |
Previously, transcluding multi-element directives (e.g. `foo-start`/`foo-end`) was not supported on elements with multi-slot transclusion (a `uterdir` error was thrown). This commit fixes it by putting the transcluded nodes into a DocumentFragment, where they can be traversed via `.nextSibling`. Fixes angular#15554
gkalpak
force-pushed
the
fix-compile-multi-element-transclusion
branch
from
5a2db25
to
f1948e2
Compare
|
LGTM with one question - does this affect performance? Creating a document fragment is more expensive than creating an array. But I assume it's okay since it only happens for slot transclusion. |
|
Yes, it olny affects directives with transclusion and yes it should be a little slower (not much though according to my totally inadequate jsperf) and I don't expect it to have any noticeable impact. |
|
We'll find out :D |
Previously, transcluding multi-element directives (e.g. `foo-start`/`foo-end`) was not supported on elements with multi-slot transclusion (a `uterdir` error was thrown). This commit fixes it by putting the transcluded nodes into a DocumentFragment, where they can be traversed via `.nextSibling`. Fixes #15554 Closes #15555
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix.
What is the current behavior? (You can also link to an open issue here)
Transcluding multi-element directives (e.g.
foo-start/foo-end) is not supported on elements with multi-slot transclusion (auterdirerror is thrown).What is the new behavior (if this is a feature change)?
Transcluded nodes are put into a DocumentFragment, where they can be traversed via
.nextSibling. This way, transcluding multi-element directives on elements with multi-slot transclusion works correctly.Does this PR introduce a breaking change?
No.
Please check if the PR fulfills these requirements
Docs have been added / updated (for bug fixes / features)Other information:
Fixes #15554.