fix(upgrade): fix empty transclusion content with AngularJS@>=1.5.8 #22167
Conversation
|
You can preview 5e5cda5 at https://pr22167-5e5cda5.ngbuilds.io/. |
|
The tests we had were sufficient (as long as we use an AngularJS version >=1.5.8). Upgrading AngularJS was my test 😛 |
5e5cda5 to
2e8c09e
Compare
|
You can preview 2e8c09e at https://pr22167-2e8c09e.ngbuilds.io/. |
a93c31f to
de6bbdb
Compare
|
I managed to get karma to run tests for multiple AngularJS versions (currently 1.5.x and 1.6.x) and then I realized I have no idea what to do with bazel 😞 @petebacondarwin, feel free to PTAL anyway 😁 |
6a6738b to
6f4cecb
Compare
|
You can preview 6f4cecb at https://pr22167-6f4cecb.ngbuilds.io/. |
6f4cecb to
3cc204d
Compare
|
You can preview 3cc204d at https://pr22167-3cc204d.ngbuilds.io/. |
| @@ -123,8 +123,14 @@ export class UpgradeHelper { | |||
| const transclude = this.directive.transclude; | |||
| const contentChildNodes = this.extractChildNodes(); | |||
| let $template = contentChildNodes; | |||
| let attachChildrenFn: angular.ILinkFn|undefined = (scope, cloneAttach) => | |||
| cloneAttach !($template, scope); | |||
| let attachChildrenFn: angular.ILinkFn = (scope, cloneAttachFn) => { | |||
There was a problem hiding this comment.
It still can be undefined, right?
There was a problem hiding this comment.
I don't see how 😕 (TypeScript agrees 😃)
It is assigned a (defined) value and is never re-assigned. It should actually be const 😁
3cc204d to
36f4f4f
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
PR Checklist
Docs have been added / updated (for bug fixes / features)PR Type
What is the current behavior?
The function provided by
ngUpgradeasparentBoundTranscludeFnwhen upgrading a component with transclusion, will break in AngularJS v1.5.8+ if no transclusion content is provided. The reason is that AngularJS will try to destroy the transclusion scope (which would not be needed any more). But since the transcluded content comes from Angular, not AngularJS, there is no transclusion scope to destroy.Issue Number: #22175
What is the new behavior?
Transclusion works even if no content is provided with AngularJS v1.5.8+.
ngUpgradeprovides a dummy scope object with a no-op$destroy()method to avoid throwing an error.Does this PR introduce a breaking change?
Other information
Fixes #22175.
This commit also upgrades AngularJS to the latest stable version.