From 2d5c83779f5d13eddb99e599250b339541c389d7 Mon Sep 17 00:00:00 2001 From: Chirayu Krishnappa Date: Thu, 26 Jun 2014 12:00:45 -0700 Subject: [PATCH] fix(ng-view): cleanup should not destroy an already destroyed scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refer https://github.com/angular/angular.dart/issues/1182 and repro https://github.com/chirayuk/sample/tree/issue_1182_leaving_a_nested_ng_view NgView's register cleanup handlers this way: _leaveSubscription = route.onLeave.listen((_) { _leaveSubscription.cancel(); // … _cleanUp(); }); When there are nested ng-views, upon a route change, the parent NgView calls it's _cleanUp() first (which destroys it's child scope) and then the child NgView attempts a cleanup. However, it's child scope is already detached due to the parent NgView cleaning up causing an exception. Stack trace is: 'package:angular/core/scope.dart': Failed assertion: line 335 pos 12: 'isAttached' is not true. STACKTRACE: #0 Scope.destroy (package:angular/core/scope.dart:335:12) #1 NgView._cleanUp (package:angular/routing/ng_view.dart:130:24) #2 NgView._show. (package:angular/routing/ng_view.dart:106:15) #3 _rootRunUnary (dart:async/zone.dart:730) #4 _ZoneDelegate.runUnary (dart:async/zone.dart:462) #5 _onRunUnary. (package:angular/core/zone.dart:113:63) #6 VmTurnZone._onRunBase (package:angular/core/zone.dart:97:16) #7 _onRunUnary (package:angular/core/zone.dart:113:17) #8 _ZoneDelegate.runUnary (dart:async/zone.dart:462) #9 _CustomizedZone.runUnary (dart:async/zone.dart:667) #10 _BaseZone.runUnaryGuarded (dart:async/zone.dart:582) #11 _BufferingStreamSubscription._sendData (dart:async/stream_impl.dart:333) #12 _BufferingStreamSubscription._add (dart:async/stream_impl.dart:263) #13 _SyncBroadcastStreamController._sendData. (dart:async/broadcast_stream_controller.dart:344) #14 _BroadcastStreamController._forEachListener (dart:async/broadcast_stream_controller.dart:297) #15 _SyncBroadcastStreamController._sendData (dart:async/broadcast_stream_controller.dart:343) #16 _BroadcastStreamController.add (dart:async/broadcast_stream_controller.dart:227) #17 Router._leaveCurrentRouteHelper (package:route_hierarchical/client.dart:654:48) #18 Router._leaveCurrentRouteHelper (package:route_hierarchical/client.dart:656:47) #19 Router._leaveCurrentRoute (package:route_hierarchical/client.dart:645:41) #20 Router._leaveOldRoutes (package:route_hierarchical/client.dart:525:30) #21 Router._processNewRoute (package:route_hierarchical/client.dart:497:27) #22 Router._route. (package:route_hierarchical/client.dart:481:29) #23 _rootRunUnary (dart:async/zone.dart:730) #24 _ZoneDelegate.runUnary (dart:async/zone.dart:462) #25 _onRunUnary. (package:angular/core/zone.dart:113:63) #26 VmTurnZone._onRunBase (package:angular/core/zone.dart:97:16) #27 _onRunUnary (package:angular/core/zone.dart:113:17) #28 _ZoneDelegate.runUnary (dart:async/zone.dart:462) #29 _CustomizedZone.runUnary (dart:async/zone.dart:667) #30 _Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:488) #31 _Future._propagateToListeners (dart:async/future_impl.dart:571) #32 _Future._completeWithValue (dart:async/future_impl.dart:331) #33 _Future._asyncComplete. (dart:async/future_impl.dart:393) #34 _rootRun (dart:async/zone.dart:723) #35 _ZoneDelegate.run (dart:async/zone.dart:453) #36 _onScheduleMicrotask. (package:angular/core/zone.dart:117:43) #37 VmTurnZone._finishTurn (package:angular/core/zone.dart:143:34) #38 VmTurnZone._onRunBase (package:angular/core/zone.dart:104:43) #39 _onRunUnary (package:angular/core/zone.dart:113:17) #40 _ZoneDelegate.runUnary (dart:async/zone.dart:462) #41 _CustomizedZone.runUnary (dart:async/zone.dart:667) #42 _BaseZone.runUnaryGuarded (dart:async/zone.dart:582) #43 _BaseZone.bindUnaryCallback. (dart:async/zone.dart:608) --- lib/routing/ng_view.dart | 5 ++++- test/routing/ng_view_spec.dart | 40 ++++++++++++++++++++++++++++------ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/lib/routing/ng_view.dart b/lib/routing/ng_view.dart index 02b9c4783..98ecf3ad5 100644 --- a/lib/routing/ng_view.dart +++ b/lib/routing/ng_view.dart @@ -127,7 +127,10 @@ class NgView implements DetachAware, RouteProvider { if (_view == null) return; _view.nodes.forEach((node) => node.remove()); - _childScope.destroy(); + + if (_childScope.isAttached) { + _childScope.destroy(); + } _view = null; _childScope = null; diff --git a/test/routing/ng_view_spec.dart b/test/routing/ng_view_spec.dart index f026b2c41..50a7d7e32 100644 --- a/test/routing/ng_view_spec.dart +++ b/test/routing/ng_view_spec.dart @@ -98,42 +98,65 @@ main() { beforeEach((TestBed tb, Router _router, TemplateCache templates) { _ = tb; router = _router; + _.rootScope.context['flag'] = true; templates.put('library.html', new HttpResponse(200, '

Library

' - '
')); + '')); templates.put('book_list.html', new HttpResponse(200, '

Books

')); templates.put('book_overview.html', new HttpResponse(200, '

Book 1234

')); templates.put('book_read.html', new HttpResponse(200, '

Read Book 1234

')); + templates.put('alt.html', new HttpResponse(200, 'alt')); }); it('should switch nested templates', async(() { Element root = _.compile(''); + microLeap(); _.rootScope.apply(); microLeap(); expect(root.text).toEqual(''); router.route('/library/all'); - microLeap(); + microLeap(); _.rootScope.apply(); microLeap(); expect(root.text).toEqual('LibraryBooks'); router.route('/library/1234'); - microLeap(); + microLeap(); _.rootScope.apply(); microLeap(); expect(root.text).toEqual('LibraryBook 1234'); // nothing should change here router.route('/library/1234/overview'); - microLeap(); + microLeap(); _.rootScope.apply(); microLeap(); expect(root.text).toEqual('LibraryBook 1234'); // nothing should change here router.route('/library/1234/read'); - microLeap(); + microLeap(); _.rootScope.apply(); microLeap(); expect(root.text).toEqual('LibraryRead Book 1234'); })); - }); + it('should not attempt to destroy and already destroyed childscope', async(() { + // This can happen with nested ng-views. Refer + // https://github.com/angular/angular.dart/issues/1182 + // and repro case + // https://github.com/chirayuk/sample/tree/issue_1182_leaving_a_nested_ng_view + Element root = _.compile(''); + microLeap(); _.rootScope.apply(); microLeap(); + + router.route('/library/1234'); + microLeap(); _.rootScope.apply(); microLeap(); + + expect(root.text).toEqual('LibraryBook 1234'); + + _.rootScope.context['flag'] = false; + microLeap(); _.rootScope.apply(); microLeap(); + router.route('/alt'); + microLeap(); _.rootScope.apply(); microLeap(); + + expect(root.text).toEqual('alt'); + })); + }); describe('Inline template ngView', () { TestBed _; @@ -193,7 +216,10 @@ class NestedRouteInitializer implements Function { 'read': ngRoute(path: '/read', view: 'book_read.html'), 'admin': ngRoute(path: '/admin', view: 'admin.html'), }) - }) + }), + 'alt': ngRoute( + path: '/alt', + view: 'alt.html'), }); } }