From aad9f7c47c6123652b01cb53a36e86c72d693d3e Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 10 Aug 2018 10:52:34 -0700 Subject: [PATCH] fix(Compiler): Allow mixing of deferred/not-deferred... ... components coming from the same library (i.e. import). Mostly involved simplifying/refactoring the bit of `DartEmitter` that created a mapping of import prefixes to use. The old method tried to use one mapping for both deferred and non-deferred libraries; my new approach uses (the already created) separate mappings. I put a bit of refactoring in to make the method(s) more understandable in the future, and a simple change to "path_util.dart" to compensate for how deferred modules are stored (importing themselves is possible). Closes #1558. Closes #1559. PiperOrigin-RevId: 208234448 --- .../_files/deferred/container_component.dart | 2 + .../container_component.template.golden | 90 ++++++++++-------- .../_files/deferred/deferred_component.dart | 6 ++ .../deferred_component.outline.golden | 7 ++ .../deferred_component.template.golden | 59 ++++++++++++ .../regression/1558_mixed_deferred_test.dart | 64 +++++++++++++ angular/CHANGELOG.md | 9 +- .../lib/src/compiler/output/dart_emitter.dart | 93 ++++++++++++------- .../lib/src/compiler/output/path_util.dart | 4 + 9 files changed, 260 insertions(+), 74 deletions(-) create mode 100644 _tests/test/regression/1558_mixed_deferred_test.dart diff --git a/_goldens/test/_files/deferred/container_component.dart b/_goldens/test/_files/deferred/container_component.dart index fc9d504526..e7f47c7f24 100644 --- a/_goldens/test/_files/deferred/container_component.dart +++ b/_goldens/test/_files/deferred/container_component.dart @@ -19,6 +19,7 @@ import 'deferred_component.dart'; Hello World + ''', directives: [ DeferredChild1Component, @@ -27,6 +28,7 @@ import 'deferred_component.dart'; DeferredChildComponentWithoutNgContent, DeferredChildComponentWithNgContent, NgIf, + NotDeferredChildComponent, SampleComponent, ], ) diff --git a/_goldens/test/_files/deferred/container_component.template.golden b/_goldens/test/_files/deferred/container_component.template.golden index 9fdbb75e31..02eaf0e43c 100644 --- a/_goldens/test/_files/deferred/container_component.template.golden +++ b/_goldens/test/_files/deferred/container_component.template.golden @@ -14,16 +14,18 @@ import 'package:angular/src/core/linker/app_view.dart'; import 'container_component.dart' as import1; import 'package:angular/src/core/linker/view_container.dart'; import 'package:angular/src/common/directives/ng_if.dart'; +import 'dart:html' as import4; +import 'deferred_component.template.dart' as import5; +import 'deferred_component.dart' as import6; import 'package:angular/src/core/render/api.dart'; -import 'package:angular/src/core/linker/view_type.dart' as import5; +import 'package:angular/src/core/linker/view_type.dart' as import8; import 'package:angular/src/core/change_detection/change_detection.dart'; -import 'dart:html' as import7; -import 'package:angular/src/core/linker/app_view_utils.dart' as import8; -import 'package:angular/src/runtime.dart' as import9; +import 'package:angular/src/core/linker/app_view_utils.dart' as import10; +import 'package:angular/src/runtime.dart' as import11; import 'package:angular/angular.dart'; import 'package:angular/src/core/linker/template_ref.dart'; -import 'deferred_component.template.dart' deferred as deflib1; import 'deferred_component.dart' deferred as deflib0; +import 'deferred_component.template.dart' deferred as deflib1; final List styles$TestContainerComponent = const []; @@ -36,16 +38,19 @@ class ViewTestContainerComponent0 extends AppView parentView, int parentIndex) : super(import5.ViewType.component, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { - rootEl = import7.document.createElement('test-container'); - _renderType ??= import8.appViewUtils.createRenderType((import9.isDevMode ? 'asset:_goldens/test/_files/deferred/container_component.dart' : null), ViewEncapsulation.None, styles$TestContainerComponent); + ViewTestContainerComponent0(AppView parentView, int parentIndex) : super(import8.ViewType.component, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { + rootEl = import4.document.createElement('test-container'); + _renderType ??= import10.appViewUtils.createRenderType((import11.isDevMode ? 'asset:_goldens/test/_files/deferred/container_component.dart' : null), ViewEncapsulation.None, styles$TestContainerComponent); setupComponentType(_renderType); } @override ComponentRef build() { final _rootEl = rootEl; - final import7.HtmlElement parentRenderNode = initViewRoot(_rootEl); + final import4.HtmlElement parentRenderNode = initViewRoot(_rootEl); final _anchor_0 = createViewContainerAnchor(); parentRenderNode.append(_anchor_0); _appEl_0 = ViewContainer(0, null, this, _anchor_0); @@ -71,6 +76,11 @@ class ViewTestContainerComponent0 extends AppView viewFactory_TestContainerComponent0(AppV } class _ViewTestContainerComponent1 extends AppView { - import7.Element _el_0; + import4.Element _el_0; AppView _compView_0; dynamic _DeferredChild1Component_0_5; - _ViewTestContainerComponent1(AppView parentView, int parentIndex) : super(import5.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { + _ViewTestContainerComponent1(AppView parentView, int parentIndex) : super(import8.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { componentType = ViewTestContainerComponent0._renderType; } @override ComponentRef build() { - _compView_0 = deflib1.viewFactory_DeferredChild1Component0(this, 0); + _compView_0 = import5.viewFactory_DeferredChild1Component0(this, 0); _el_0 = _compView_0.rootEl; - _DeferredChild1Component_0_5 = deflib0.DeferredChild1Component(); + _DeferredChild1Component_0_5 = import6.DeferredChild1Component(); _compView_0.create(_DeferredChild1Component_0_5, []); init0(_el_0); return null; @@ -142,7 +154,7 @@ AppView viewFactory_TestContainerComponent1(AppV class _ViewTestContainerComponent2 extends AppView { ViewContainer _appEl_0; - _ViewTestContainerComponent2(AppView parentView, int parentIndex) : super(import5.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { + _ViewTestContainerComponent2(AppView parentView, int parentIndex) : super(import8.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { componentType = ViewTestContainerComponent0._renderType; } @override @@ -166,17 +178,17 @@ AppView viewFactory_TestContainerComponent2(AppV } class _ViewTestContainerComponent3 extends AppView { - import7.Element _el_0; + import4.Element _el_0; AppView _compView_0; dynamic _DeferredChild2Component_0_5; - _ViewTestContainerComponent3(AppView parentView, int parentIndex) : super(import5.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { + _ViewTestContainerComponent3(AppView parentView, int parentIndex) : super(import8.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { componentType = ViewTestContainerComponent0._renderType; } @override ComponentRef build() { - _compView_0 = deflib1.viewFactory_DeferredChild2Component0(this, 0); + _compView_0 = import5.viewFactory_DeferredChild2Component0(this, 0); _el_0 = _compView_0.rootEl; - _DeferredChild2Component_0_5 = deflib0.DeferredChild2Component(); + _DeferredChild2Component_0_5 = import6.DeferredChild2Component(); _compView_0.create(_DeferredChild2Component_0_5, []); init0(_el_0); return null; @@ -198,14 +210,14 @@ AppView viewFactory_TestContainerComponent3(AppV } class _ViewTestContainerComponent4 extends AppView { - import7.DivElement _el_0; + import4.DivElement _el_0; ViewContainer _appEl_1; - _ViewTestContainerComponent4(AppView parentView, int parentIndex) : super(import5.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { + _ViewTestContainerComponent4(AppView parentView, int parentIndex) : super(import8.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { componentType = ViewTestContainerComponent0._renderType; } @override ComponentRef build() { - var doc = import7.document; + var doc = import4.document; _el_0 = doc.createElement('div'); final _anchor_1 = createViewContainerAnchor(); _el_0.append(_anchor_1); @@ -227,17 +239,17 @@ AppView viewFactory_TestContainerComponent4(AppV } class _ViewTestContainerComponent5 extends AppView { - import7.Element _el_0; + import4.Element _el_0; AppView _compView_0; dynamic _DeferredChild3Component_0_5; - _ViewTestContainerComponent5(AppView parentView, int parentIndex) : super(import5.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { + _ViewTestContainerComponent5(AppView parentView, int parentIndex) : super(import8.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { componentType = ViewTestContainerComponent0._renderType; } @override ComponentRef build() { - _compView_0 = deflib1.viewFactory_DeferredChild3Component0(this, 0); + _compView_0 = import5.viewFactory_DeferredChild3Component0(this, 0); _el_0 = _compView_0.rootEl; - _DeferredChild3Component_0_5 = deflib0.DeferredChild3Component(); + _DeferredChild3Component_0_5 = import6.DeferredChild3Component(); _compView_0.create(_DeferredChild3Component_0_5, []); init0(_el_0); return null; @@ -250,7 +262,7 @@ class _ViewTestContainerComponent5 extends AppView(parentView.parentView)._query_queryMe_1_0_isDirty = true; + import11.unsafeCast(parentView.parentView)._query_queryMe_1_0_isDirty = true; } @override @@ -264,18 +276,18 @@ AppView viewFactory_TestContainerComponent5(AppV } class _ViewTestContainerComponent6 extends AppView { - import7.Element _el_0; + import4.Element _el_0; AppView _compView_0; dynamic _DeferredChildComponentWithoutNgContent_0_5; - _ViewTestContainerComponent6(AppView parentView, int parentIndex) : super(import5.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { + _ViewTestContainerComponent6(AppView parentView, int parentIndex) : super(import8.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { componentType = ViewTestContainerComponent0._renderType; } @override ComponentRef build() { - _compView_0 = deflib1.viewFactory_DeferredChildComponentWithoutNgContent0(this, 0); + _compView_0 = import5.viewFactory_DeferredChildComponentWithoutNgContent0(this, 0); _el_0 = _compView_0.rootEl; - _DeferredChildComponentWithoutNgContent_0_5 = deflib0.DeferredChildComponentWithoutNgContent(); - import7.Text _text_1 = import7.Text('Hello World'); + _DeferredChildComponentWithoutNgContent_0_5 = import6.DeferredChildComponentWithoutNgContent(); + import4.Text _text_1 = import4.Text('Hello World'); _compView_0.create(_DeferredChildComponentWithoutNgContent_0_5, []); init0(_el_0); return null; @@ -297,18 +309,18 @@ AppView viewFactory_TestContainerComponent6(AppV } class _ViewTestContainerComponent7 extends AppView { - import7.Element _el_0; + import4.Element _el_0; AppView _compView_0; dynamic _DeferredChildComponentWithNgContent_0_5; - _ViewTestContainerComponent7(AppView parentView, int parentIndex) : super(import5.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { + _ViewTestContainerComponent7(AppView parentView, int parentIndex) : super(import8.ViewType.embedded, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { componentType = ViewTestContainerComponent0._renderType; } @override ComponentRef build() { - _compView_0 = deflib1.viewFactory_DeferredChildComponentWithNgContent0(this, 0); + _compView_0 = import5.viewFactory_DeferredChildComponentWithNgContent0(this, 0); _el_0 = _compView_0.rootEl; - _DeferredChildComponentWithNgContent_0_5 = deflib0.DeferredChildComponentWithNgContent(); - import7.Text _text_1 = import7.Text('Hello World'); + _DeferredChildComponentWithNgContent_0_5 = import6.DeferredChildComponentWithNgContent(); + import4.Text _text_1 = import4.Text('Hello World'); _compView_0.create(_DeferredChildComponentWithNgContent_0_5, [ [_text_1] ]); @@ -336,7 +348,7 @@ final List styles$TestContainerComponentHost = const []; class _ViewTestContainerComponentHost0 extends AppView { ViewTestContainerComponent0 _compView_0; import1.TestContainerComponent _TestContainerComponent_0_5; - _ViewTestContainerComponentHost0(AppView parentView, int parentIndex) : super(import5.ViewType.host, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways); + _ViewTestContainerComponentHost0(AppView parentView, int parentIndex) : super(import8.ViewType.host, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways); @override ComponentRef build() { _compView_0 = ViewTestContainerComponent0(this, 0); diff --git a/_goldens/test/_files/deferred/deferred_component.dart b/_goldens/test/_files/deferred/deferred_component.dart index 1207f623d5..515b6c091a 100644 --- a/_goldens/test/_files/deferred/deferred_component.dart +++ b/_goldens/test/_files/deferred/deferred_component.dart @@ -18,6 +18,12 @@ class DeferredChild2Component {} ) class DeferredChild3Component {} +@Component( + selector: 'not-deferred-child', + template: '', +) +class NotDeferredChildComponent {} + @Component( selector: 'deferred-child-without-ng-content', template: '
Child
', diff --git a/_goldens/test/_files/deferred/deferred_component.outline.golden b/_goldens/test/_files/deferred/deferred_component.outline.golden index be823922fd..3e1ce4e6f8 100644 --- a/_goldens/test/_files/deferred/deferred_component.outline.golden +++ b/_goldens/test/_files/deferred/deferred_component.outline.golden @@ -34,6 +34,13 @@ external AppView<_user.DeferredChild3Component> viewFactory_DeferredChild3Compon class ViewDeferredChild3Component0 extends AppView<_user.DeferredChild3Component> { external ViewDeferredChild3Component0(AppView parentView, int parentIndex); } +// For @Component class NotDeferredChildComponent. +external List get styles$NotDeferredChildComponent; +external ComponentFactory<_user.NotDeferredChildComponent> get NotDeferredChildComponentNgFactory; +external AppView<_user.NotDeferredChildComponent> viewFactory_NotDeferredChildComponent0(AppView parentView, int parentIndex); +class ViewNotDeferredChildComponent0 extends AppView<_user.NotDeferredChildComponent> { + external ViewNotDeferredChildComponent0(AppView parentView, int parentIndex); +} // For @Component class DeferredChildComponentWithoutNgContent. external List get styles$DeferredChildComponentWithoutNgContent; external ComponentFactory<_user.DeferredChildComponentWithoutNgContent> get DeferredChildComponentWithoutNgContentNgFactory; diff --git a/_goldens/test/_files/deferred/deferred_component.template.golden b/_goldens/test/_files/deferred/deferred_component.template.golden index 75de743584..5ee155ad98 100644 --- a/_goldens/test/_files/deferred/deferred_component.template.golden +++ b/_goldens/test/_files/deferred/deferred_component.template.golden @@ -198,6 +198,64 @@ ComponentFactory get DeferredChild3ComponentNgF return _DeferredChild3ComponentNgFactory; } +final List styles$NotDeferredChildComponent = const []; + +class ViewNotDeferredChildComponent0 extends AppView { + static RenderComponentType _renderType; + ViewNotDeferredChildComponent0(AppView parentView, int parentIndex) : super(import3.ViewType.component, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways) { + rootEl = import5.document.createElement('not-deferred-child'); + _renderType ??= import6.appViewUtils.createRenderType((import7.isDevMode ? 'asset:_goldens/test/_files/deferred/deferred_component.dart' : null), ViewEncapsulation.None, styles$NotDeferredChildComponent); + setupComponentType(_renderType); + } + @override + ComponentRef build() { + final _rootEl = rootEl; + final import5.HtmlElement parentRenderNode = initViewRoot(_rootEl); + init(const [], null); + return null; + } +} + +AppView viewFactory_NotDeferredChildComponent0(AppView parentView, int parentIndex) { + return ViewNotDeferredChildComponent0(parentView, parentIndex); +} + +final List styles$NotDeferredChildComponentHost = const []; + +class _ViewNotDeferredChildComponentHost0 extends AppView { + ViewNotDeferredChildComponent0 _compView_0; + import1.NotDeferredChildComponent _NotDeferredChildComponent_0_5; + _ViewNotDeferredChildComponentHost0(AppView parentView, int parentIndex) : super(import3.ViewType.host, {}, parentView, parentIndex, ChangeDetectionStrategy.CheckAlways); + @override + ComponentRef build() { + _compView_0 = ViewNotDeferredChildComponent0(this, 0); + rootEl = _compView_0.rootEl; + _NotDeferredChildComponent_0_5 = import1.NotDeferredChildComponent(); + _compView_0.create(_NotDeferredChildComponent_0_5, projectableNodes); + init0(rootEl); + return ComponentRef(0, this, rootEl, _NotDeferredChildComponent_0_5); + } + + @override + void detectChangesInternal() { + _compView_0.detectChanges(); + } + + @override + void destroyInternal() { + _compView_0?.destroy(); + } +} + +AppView viewFactory_NotDeferredChildComponentHost0(AppView parentView, int parentIndex) { + return _ViewNotDeferredChildComponentHost0(parentView, parentIndex); +} + +const ComponentFactory _NotDeferredChildComponentNgFactory = const ComponentFactory('not-deferred-child', viewFactory_NotDeferredChildComponentHost0); +ComponentFactory get NotDeferredChildComponentNgFactory { + return _NotDeferredChildComponentNgFactory; +} + final List styles$DeferredChildComponentWithoutNgContent = const []; class ViewDeferredChildComponentWithoutNgContent0 extends AppView { @@ -333,6 +391,7 @@ void initReflector() { _ngRef.registerComponent(DeferredChild1Component, DeferredChild1ComponentNgFactory); _ngRef.registerComponent(DeferredChild2Component, DeferredChild2ComponentNgFactory); _ngRef.registerComponent(DeferredChild3Component, DeferredChild3ComponentNgFactory); + _ngRef.registerComponent(NotDeferredChildComponent, NotDeferredChildComponentNgFactory); _ngRef.registerComponent(DeferredChildComponentWithoutNgContent, DeferredChildComponentWithoutNgContentNgFactory); _ngRef.registerComponent(DeferredChildComponentWithNgContent, DeferredChildComponentWithNgContentNgFactory); _ref0.initReflector(); diff --git a/_tests/test/regression/1558_mixed_deferred_test.dart b/_tests/test/regression/1558_mixed_deferred_test.dart new file mode 100644 index 0000000000..1a9dd3f66f --- /dev/null +++ b/_tests/test/regression/1558_mixed_deferred_test.dart @@ -0,0 +1,64 @@ +@TestOn('browser') +import 'package:angular/angular.dart'; +import 'package:angular_test/angular_test.dart'; +import 'package:test/test.dart'; + +import '1558_mixed_deferred_test.template.dart' as ng; + +// See https://github.com/dart-lang/angular/issues/1558. +void main() { + tearDown(disposeAnyRunningTest); + + test('should allow deferring some components from an import', () async { + // Previously, if you had library "a.dart", and it had Comp1 and Comp2, and + // Comp1 was imported and used with "@deferred", and Comp2 was not used with + // "@deferred", a compiler error would be thrown by DDC/Dart2JS: + // "The deferred type deflib0.Comp2 can't be used in a ..." + // + // This is because the emitter would try re-using the "deferred as" import + // (generated by the compiler) to refer to types (including Comp2) that were + // not meant to be deferred. + await NgTestBed.forComponent(ng.MixedDeferredTest1NgFactory).create(); + }); + + test('should allow deferring in some components and not others', () async { + // MixedDeferredTest1 defer loads Comp1, but MixedDeferredTest2 does not. + await NgTestBed.forComponent(ng.MixedDeferredTest2NgFactory).create(); + }); +} + +@Component( + selector: 'mixed-deferred-test', + template: r''' + + + ''', + directives: [ + Comp1, + Comp2, + ], +) +class MixedDeferredTest1 {} + +@Component( + selector: 'mix-deferred-test', + template: r''' + + ''', + directives: [ + Comp1, + ], +) +class MixedDeferredTest2 {} + +@Component( + selector: 'comp-1', + template: '', +) +class Comp1 {} + +@Component( + selector: 'comp-2', + template: '', +) +class Comp2 {} diff --git a/angular/CHANGELOG.md b/angular/CHANGELOG.md index 9da6e5899a..3cfc496b35 100644 --- a/angular/CHANGELOG.md +++ b/angular/CHANGELOG.md @@ -1,6 +1,6 @@ ### Bug fixes -* [#1538]() A compile-time error is reported if the `@deferred` template +* [#1538][]: A compile-time error is reported if the `@deferred` template annotation is present on a `