fix(runtime): do regular clone of normal slotting #2694
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Issue #2257 might be related. |
justusromijn
changed the title
|
I've included a test scenario as requested. Its not perfect with the inclusion of the angular library, I could not find a way to prevent double loading when running karma tests (but if I do not load it from the karma config, it seems to also not load from the index file itself). Maybe the Ionic Team can help me out there? |
adamdbradley
pushed a commit
that referenced
this pull request
Oct 19, 2020
Co-Authored-By: Justus Romijn <1901845+justusromijn@users.noreply.github.com>
adamdbradley
pushed a commit
that referenced
this pull request
Oct 22, 2020
Co-Authored-By: Justus Romijn <1901845+justusromijn@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The extra option
cloneNodehas a drawback for angularJS. When an ng-if is true at its initial render, the originally slotted content is not visible. This is not a problem for ng-if blocks that start out with a falsy value and are appearing later.By adding a check to not patch up any non-stencil nodes, this problem seems resolved.
I would like to get some feedback from the maintainers whether this has a chance of making the codebase, it is quite a blocker for our team to upgrade to 1.8.x and higher.
I could not find any tests related to the dom-extra's, please point me in the correct direction if this needs proper coverage.
Here's a demo repository with a stencil generated app (1.8.x), loading a very basic angularJS binding context to demonstrate the problem. In the readme are the steps to reproduce the different problems, and a way to apply the patch of this PR to see the solution.