Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UpgradeModule needs to support Hybrid Apps w/ Lazily loaded Hybrid Bundles, using the AngularJS Router #17490

Open
aaronfrost opened this issue Jun 14, 2017 · 28 comments
Labels
area: upgrade Issues related to AngularJS → Angular upgrade APIs feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature freq1: low P4 A relatively minor issue that is not relevant to core functions workaround2: non-obvious
Milestone

Comments

@aaronfrost
Copy link
Contributor

aaronfrost commented Jun 14, 2017

I'm submitting a ...


[ ] Regression (behavior that used to work and stopped working in a new release)
[ ] Bug report 
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

I am able to lazily load code in an AngularJS app that uses the AngularJS router, using ocLazyLoad. This all works. If you add Angular components to both the main app, as well as to the lazily loaded feature bundle, the Angular components in the main app are accessible to be downgraded and used inside of AngularJS templates. However, the Angular components in the lazily loaded feature bundle are able to be downgraded, but NOT able to be used in the AngularJS templates. This is due largely to the fact that the downgradeComponent method doesn't allow me to specify which injector it should use to access the componentFactories for the lazily loaded components. Because I can't specify which injector to use, it uses the main AppModule's injector, which has no knowledge of the lazily loaded module or components.

Expected behavior

I should be able to optionally specify which injector I want the downgradeComponent to use to resolve the componentFactory.

Minimal reproduction of the problem with instructions

  1. Create an AngularJS app, that uses the AngularJS router.
  2. Add a webpack build to your project that will allow you to easily code split, so that you can break your lazily loaded features in to separate bundles.
  3. Add ocLazyLoad to the app, and use it to lazily load a feature/section of the app.
  4. Add Angular to the app (npm i @angular/common @angular/compiler @angular/compiler-cli @angular/core @angular/forms @angular/http @angular/platform-browser @angular/platform-browser-dynamic @angular/router @angular/upgrade rxjs zone.js)
  5. Create an AppModule that will be part of your main bundle. Add a simple component to that module, and downgrade it, and use it in your AngularJS template, to verify that Angular is working inside of the AngularJS app. As part of this step, you will need to bootstrap the Angular app first, and then in the callback from that, using the UpgradeModule you bootstrap the AngularJS app.
  6. Now add a FooModule to your lazily loaded feature bundle, and add a FooComponent to that modules entryComponents. Make sure and downgrade this component so that it can be used in the AngularJS pieces of this lazily loaded feature.
  7. Add the downgraded component to the AngularJS template. (if you refresh the page at this point, you will get an error: No component factory found for FooComponent. Did you add it to @NgModule.entryComponents?, but keep going)
  8. Create a separate tsconfig-aot.json and in the files value, add a reference to the file containing your FooModule.
  9. Run ngc -p path/to/your/tsconfig-aot.json to generate your 'FooModuleNgFactory`.
  10. In the bundle for your lazily loaded feature, import the FooModuleNgFactory and call the following:
const appModuleRef = getAppModuleRef(); // get access to it somehow
const fooModuleRef = FooModuleNgFactory.create(appModuleRef.injector);

// now that you created an instance of your lazily loaded module, you should downgrade the component in that module
angular
    .module('angularJSModule')
    .directive('fooComponent', downgradeComponent({component:FooComponent});

Having done all of this, I expected that the components from my lazily loaded Angular module would be able to work. But when I add them to my AngularJS templates I get the error No component factory found for FooComponent. Did you add it to @NgModule.entryComponents?. The reason that it is throwing this error is because the call to downgradeComponent is trying to find the FooComponentFactory in the main AppModule's injector. But it doesn't exist in that injector. It exists in the child injector that was created when I created the instance of my lazily loaded module (ie: when I called FooModuleNgFactory.create).

If I hack the Angular source to allow me to optionally pass in the injector to the downgradeComponent method, it all works fine. Just like I would expect.

I would like to see if the UpgradeModule's downgradeComponent method could support an optional injector to be passed in. The hack that I made to get it to work was the following:

const appModuleRef = getAppModuleRef();
const fooModuleRef = FooModuleNgFactory.create(appModuleRef.injector);
const directiveConfig = downgradeComponent({component:FooComponent, injector: fooModuleRef.injector});

angular
    .module('angularJsModule')
    .directive('fooComponent', directiveConfig);

Notice how I pass in the injector as well. I modified the downgradeComponent function to look like this:
image

Doing this, I was able to get Angular modules lazily loaded and created WITHOUT using the Angular router. It would be nice if the downgradeComponent method supported it native.

I spoke with @robwormald, and he agreed that this is a valid use-case that ngUpgrade should cover. He said he may speak with @petebacondarwin about it to see what his thoughts are.

What is the motivation / use case for changing the behavior?

All of the examples in the ngUpgrade documentation are under the assumption that I am using the Angular router. Many apps are very large, and aren't able to switch their router before they even have any Angular components in that lazily loaded section of code. It makes sense for them to need to continue using the AngularJS router.

Please tell us about your environment


Angular version: 1.5.11


Browser:
- [x] Chrome (desktop) version XX
- [x] Chrome (Android) version XX
- [x] Chrome (iOS) version XX
- [x] Firefox version XX
- [x] Safari (desktop) version XX
- [x] Safari (iOS) version XX
- [x] IE version XX
- [x] Edge version XX
 
For Tooling issues:
- Node version: 7.4.0 
- Platform: Mac 

Others:

@aaronfrost aaronfrost changed the title UpgradeModule needs to support Hybrid Apps w/ Hybrid Bundles, using the AngularJS Router UpgradeModule needs to support Hybrid Apps w/ Lazily loaded Hybrid Bundles, using the AngularJS Router Jun 14, 2017
@petebacondarwin
Copy link
Member

I agree that this sounds like a scenario that we should support. My first feeling is that this approach looks good.

Once the module has been lazily loaded AngularJS expects the component to be available everywhere - since this is how AngularJS works. Are there any issues with the implementation of the component only being available in the lazy loaded Angular module's injector?

@aaronfrost
Copy link
Contributor Author

aaronfrost commented Jun 15, 2017

I am very likely incorrect, but here are my thoughts. Because the module is lazily loaded, the main non-lazily loaded app won't have any idea about the components in that module. If those nonLL pieces depended on anything from the LL bundle, then they would have to import them, which would mean that those pieces wouldn't be LL anymore. So... it seems to me, based on the nature of LL components, that only the bundle that is LL'd will have need to talk to those components and that other parent or sibling injectors won't have a need to access the components inside of the injector of the LL'd module.

Even in AngularJS this is how it works. The pieces that are NOT lazy loaded do not use the pieces that are lazy loaded. If they used them, then they would get an error, cause they haven't been loaded yet. Thus... the nature of it is that lazy loaded pieces are able to use existing components, but existing components are not able to use lazily loaded components.

I say this with only a high level understanding of how Angular module/injector hierarchy works. The bulk of my understanding is coming from the concepts I learned when LL'ing AngularJS code.

Thoughts?

@samudrak
Copy link

samudrak commented Jul 20, 2017

@aaronfrost we are also stuck at same situation , we are unable to specify which injector I want the downgradeComponent to use to resolve the componentFactory.

Suppose if you uses routing will it automatically take injector of the loaded module by the Router as the parent injector.

At the moment i am using system module loader to load the dynamically created module in a given path.

image

Any new ideas are welcome

@rgullhaug
Copy link

@petebacondarwin Will this be implemented as suggested by @aaronfrost ? We have tested the code he wrote and it seems to work fine, but we don't want to go forward with it before we know that it will be part of the official angular implementation.

@samudrak
Copy link

samudrak commented Sep 6, 2017

@petebacondarwin , any update on this issue . Will this be implemented as suggested by @aaronfrost ?

As @rgullhaug mentioned in his previous reply this change works for us , but don't wanted to go forward until you confirm this will be available in the official angular implementation .

@petebacondarwin
Copy link
Member

We are still looking into this...

We definitely want to support this use case.

There is a new API for an alternative way of bootstrapping upgrade apps called downgradeModule, where the Angular side of the app is only bootstrapped when the first downgraded Component is instantiated in an AngularJS template. SInce in this approach we don't control when the downgraded module is bootstrapped we can't tell the downgraded component what its injector should be. So we need to think about how this would work for that.

For apps using UpgradeModule to bootstrap, this approach does work, even if it requires some boilerplate, which it would be nice to remove. But since both UpgradeModule and downgradeModule share the use of downgradeComponent API we need to make sure that things don't get any more crazy than they already are!

@petebacondarwin
Copy link
Member

petebacondarwin commented Sep 11, 2017

Here is an idea... as a workaround, which should be possible right now without any changes to ngUpgrade.

How about you create a directive, or a route resolve handler, which will attach the newly loaded Angular module's injector to the DOM as a data item with the INJECTOR_KEY ("$injector")? The current implementation of downgradeComponent will always use such a parentInjector before looking up to the root injector.

@petebacondarwin
Copy link
Member

@samudrak
Copy link

samudrak commented Sep 12, 2017

Thanks a lot @petebacondarwin . Got it working as per your workaround by attach the newly loaded Angular module's injector to the DOM as a data item with the INJECTOR_KEY

@petebacondarwin
Copy link
Member

I'm interested to discuss approaches for doing this attaching in different scenarios. @samudrak is your attaching being done in a router resolve somewhere? Or another approach?

@samudrak
Copy link

samudrak commented Sep 13, 2017

@petebacondarwin i tested it with two different scenarios .

1. [ SCENARIO 1] in the first scenario i attached the injector in a router resolver.

app.config(['$routeProvider', '$controllerProvider', '$compileProvider', '$filterProvider', '$provide', '$qProvider', '$locationProvider', '$httpProvider',
        function ($routeProvider, $controllerProvider, $compileProvider, $filterProvider, $provide, $qProvider, $locationProvider, $httpProvider){
       app.register =
                {
                    controller: $controllerProvider.register,
                    directive: $compileProvider.directive,
                    filter: $filterProvider.register,
                    factory: $provide.factory,
                    service: $provide.service,
                    component: $compileProvider.component
                };
           $routeProvider.when(
             '/',{
             templateUrl: 'resolveView.html',
            controller: 'resolveCtrl'
           }).otherwise({
            template: " ",
            controller: 'resolveCtrl',
            resolve: {
                resolvedVal: ['$q', '$timeout', '$rootElement', '$compile', '$rootScope', function ($q, $timeout, $rootElement, $compile, $rootScope) {
                  var deferred = $q.defer();
                  app.loader.load("app/custom.module").then((modFac) => {
                          console.log("After Module Loaded  ");
                          app.modFactory = app.compiler.compileModuleAndAllComponentsSync(modFac.moduleType);
                          var ngModFactory = app.modFactory.ngModuleFactory;
                          app.childMod = ngModFactory.create(app.modInjector);
                          console.log(app.modFactory.componentFactories);
                          app.factoryResolver = app.childMod.injector.get(ComponentFactoryResolver) as ComponentFactoryResolver
                          
                         app.register.directive("rgTypescriptTestComponent", app.downgradeComponent({ component: app.modFactory.componentFactories[72].componentType}));
                          const newElement = angular.element('<div><rg-typescript-test-component></rg-typescript-test-component></div>');
                          newElement.data('$$$angularInjectorController', app.childMod.injector);
                          $rootElement.append(newElement);
                          $compile(newElement)($rootScope);
                          $timeout(function() {
                        deferred.resolve();
                    });
                  });
                  console.log('Before Promise Return');
                    return deferred.promise;
                }]
            }});
        }]);

2. [SCENARIO 2] In the second scenario i am attaching the injector when the DOM is ready . Here first i fully load the DOM with empty div's(placeholder div's ) for each component and does the following when the DOM is ready.

**// 1 . Injector for the module** 
var moduleInject = applicationService.getModuleIjnector();
**// 2. Get the placeholder  div** 
var placeholderElement = $('#rgtypescripttestContainer');
**// 3. Create child element with test component** 
const newElementChield = angular.element(`<rg-typescript-test-component></rg-typescript-test-component>`);
**// 4. attach it to placeholder** 
placeholderElement.append(newElementChield);
**// 5.  attach the injector to placeholder div** 
placeholderElement.data('$$$angularInjectorController', moduleInject);
**//6 recompile the placeholder with $rootScope** 
$compile(placeholderElement)($rootScope);

@petebacondarwin
Copy link
Member

Cool! So the key thing I am thinking about is where we need to attach the injector in different situations. In my PoC I was attaching the newly loaded component to the root element (via a container, which holds the new injector), but in practice I guess this is not what will happen.

If you are loading the new code in a route, then I expect that the new components will only appear in this route's template and so it might be enough to attach the injector at the ng-view/'ui-view element?

@samudrak
Copy link

samudrak commented Sep 14, 2017

absolutely . you are spot on @petebacondarwin . other wise in the downgrademodule it will unnecessarily traverse up in the DOM tree until it reach the root element to find the new injector .

In my case i have a container(place holder ) which hold the all the components for my application . So i am attaching injector to that place holder .

As a general practice we can say container which holds ng-view/'ui-view is the correct place to attach the new injector.

May be when we compile instead using $rootScope we should use a scope associated to container, which holds the new injector ?

@brianmcd
Copy link

@petebacondarwin - Thanks for sharing your workaround. It's working great on my team (1.6/5 hybrid using UpgradeModule). We converted from ui-router to the angular 5 router while getting the hybrid app running, so our setup is slightly different than your POC. For lazy-loaded routes, we're attaching the lazy module's injector to the element of the root component of that route in its ngOnInit. So far, so good!

@FDIM
Copy link
Contributor

FDIM commented Jan 29, 2018

I am trying to use the workaround and it seems to work (by setting $$$angularInjectorController to module ref/injector on view container) but I am facing another issue is that this way needsNgZone remains false in downgradeComponent function, thus in turn digest or zone is not triggered and DOM doesn't get updated after user actions.

Perhaps default value could be passed via info param? I can submit a PR with this tiny feature if it would be accepted: FDIM@0022d35

@gkalpak
Copy link
Member

gkalpak commented Jan 29, 2018

@FDIM, are you using downgradeModule()? If not, aren't lazy-loaded modules run inside the Zone anyway?

Could you share a minimal reproduction of the issue (either as a repo we can check out or a live exampe on StackBlitz)?
(You could use this project as a starting point.)

@FDIM
Copy link
Contributor

FDIM commented Jan 29, 2018

@gkalpak Yep, we are using downgradeModule, but only for the main application module, not for lazy loaded modules.

Aside of that, when lazy angular 5 module is loaded, it also comes with angular 1 module, which is then initialized via $injector.loadNewModules introduced in angularjs 1.6.7 and it includes downgraded components (the ones that fail to initialize).

All routes are set in main application bundle and splitting is done like described here (fake lazy route): angular/angular-cli#9343

I'll prepare stackblitz example tomorrow if you still need it.

@FDIM
Copy link
Contributor

FDIM commented Jan 30, 2018

@gkalpak I've prepared a repository where the issue I mentioned can be reproduced: https://github.com/FDIM/angular-hybrid-lazy-load-example

It is a regular cli project with the complete example visible in 2nd commit: FDIM/angular-hybrid-lazy-load-example@918d750

Eventually when you run npm start and open localhost:4200 you will see a link to go to a route that loads lazy module. After that there will be a div element that once clicked should show/hide another element. Clicking it now has no affect unless angular.element(document.body).data('$scope').$apply(); is called (or something else triggers digest) or a fix is made in downgradeComponent in @angular/upgrade/static.

I am not sure what the actual fix should be, but to test you can open developer tools, lookup static.js file, locate downgradeComponent and put a breakpoint inside link function (line 519 for me) and change needsNgZone to true when component in lazy module is invoked. If the value is true, component will work as intended.

I am on gitter if you have any questions about this.

@gkalpak
Copy link
Member

gkalpak commented Jan 30, 2018

Thx for the repro, @FDIM. I don't think I'll get to it this week, but I'll try to take a look early next week.

@ngbot ngbot bot removed this from the Backlog milestone Feb 28, 2018
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Feb 28, 2018
@FDIM
Copy link
Contributor

FDIM commented Mar 2, 2018

@gkalpak How about adding another require ?^^$$angularInjectorNeedsNgZone and use that value instead of default false? This way a workaround mentioned here would work for both cases. Would PR with such change be acceptable?
image

EDIT: Since the issue I am having is not directly related to this, I've created another one: #22581

@akjana35
Copy link

akjana35 commented Aug 3, 2018

any updates on this issue for an official solution ? We have the same problem, the workaround mentioned by petebacondarwin works. Thanks!

@gkalpak gkalpak added area: upgrade Issues related to AngularJS → Angular upgrade APIs and removed area: upgrade Issues related to AngularJS → Angular upgrade APIs comp: upgrade/static labels Mar 13, 2019
@richieforeman
Copy link

This would be a very interesting issue to solve.

As of right now, in an UpgradeModule application, downgradeComponent can only be used to expose components that are available in the root ComponentFactoryResolver. If you have multiple ComponentFactoryResolvers (e.g. If you're lazy loading), this requires you to register the components you'd like to downgradeComponent on within the root injector/module.

Additional context, this would be possible with downgradeModule, but that's not available for applications that use UpgradeModule

@anvlkv
Copy link

anvlkv commented Aug 20, 2019

@brianmcd and @petebacondarwin Thank you so much!

@jelbourn jelbourn added P4 A relatively minor issue that is not relevant to core functions and removed severity2: inconvenient labels Oct 1, 2020
@dwilches
Copy link

dwilches commented Jan 8, 2021

@brianmcd
About this:

we're attaching the lazy module's injector to the element of the root component of that route in its ngOnInit. So far, so good!

Could you share the code that shows how to attach the injector?
I also am migrating from UI-Router into the new Angular Router.

Thanks.

@brianmcd
Copy link

@dwilches - I wish I could help, but I've long since rolled off of that project, so I don't have access to the code or remember the details of how it worked.

@dwilches
Copy link

@dwilches - I wish I could help, but I've long since rolled off of that project, so I don't have access to the code or remember the details of how it worked.

Thanks, no problem, I figured out my issue: https://stackoverflow.com/questions/65669424/angular-hybrid-typeerror-cannot-read-property-get-of-undefined

@petebacondarwin petebacondarwin removed their assignment Jan 12, 2021
@angular-robot angular-robot bot added the feature: under consideration Feature request for which voting has completed and the request is now under consideration label Jun 4, 2021
@blemaire
Copy link

blemaire commented Dec 3, 2021

Hi all, first of all, thanks a lot for all the pointers present in this issue.

I have managed to get something working. It's a little hacky but not that much.

Angular service used to lazyload Angular modules and fire events when completed

import {Compiler, Injectable, Injector, NgModuleFactory, NgModuleRef, Type} from '@angular/core';
import {Observable, of, Subject} from 'rxjs';
import {filter, first} from 'rxjs/operators';

export interface ILazyLoadedModule<T> {
    moduleName: string,
    module: NgModuleRef<T>
}

@Injectable({
    providedIn: 'root',
})
export class AngularModuleLoaderService {
    private loadedModules = new Map<string, NgModuleRef<any>>();

    private moduleLoadEvent = new Subject<ILazyLoadedModule<any>>();

    constructor(
        private injector: Injector,
        private compiler: Compiler,
    ) {
        this.moduleLoadEvent.subscribe(loadedModule => {
            if (!this.loadedModules.has(loadedModule.moduleName)) {
                this.loadedModules.set(loadedModule.moduleName, loadedModule.module);
            }
        });
    }

    // Used to load an angular module
    public loadModuleSync<T>(moduleName: string, moduleFactory: NgModuleFactory<T> | Type<T>): NgModuleRef<T> {
        if (moduleFactory instanceof NgModuleFactory) {
            // For AOT

        } else {
            // For JIT
            moduleFactory = this.compiler.compileModuleSync(moduleFactory);
        }

        const module = moduleFactory.create(this.injector);
        this.markModuleLoaded(moduleName, module);

        return module;
    }

    // Observable used to detect when an angular module has finished loading
    public moduleLoaded<T>(moduleName: string): Observable<ILazyLoadedModule<T>> {
        if (this.loadedModules.has(moduleName)) {
            return of({moduleName, module: this.loadedModules.get(moduleName)});
        }

        return this.moduleLoadEvent.pipe(
            filter(module => module.moduleName === moduleName),
            first(),
        );
    }

    // Trigger the event to say the module has finished being loaded
    private markModuleLoaded<T>(moduleName: string, module: NgModuleRef<T>): void {
        this.moduleLoadEvent.next({moduleName, module});
    }
}

AngularJs directive and services required

angular.module('ng-module-downgrade-module', [])
     // The downgraded service used to detect when the angular module has finished loading
     .factory('NgModuleLoader', downgradeInjectable(AngularModuleLoaderService))
    .directive('moduleLazyLoad', /* ngInject */ function (NgModuleLoader, $compile) {
        return {
            restrict: 'A',
            terminal: true,
            compile: function () {
                return function ($scope, $element, $attr) {
                    // Wait for the angular module to load
                    NgModuleLoader.moduleLoaded($attr.moduleLazyLoad).subscribe((loadedModule) => {
                        // HACK !!
                        // make the newly loaded Angular module available for the downgraded component.
                        // This will ensure the downgraded component link function has the correct injector
                        $element.data('$$$angularInjectorController', loadedModule.module.injector);

                        // Compile the original content of the component.
                        $compile($element.contents())($scope);
                    });
                };
            },
        };
    })
   // this is used to load the Angular module (can be done in a separate module (even an angularJs lazy loaded module)
   .run(/* @ngInject */(NgModuleLoader) => {
        NgModuleLoader.loadModuleSync('LazyLoadedAngularModule', AngulaModule);
    })
   // Downgrade the component from the lazyloaded angular module
    .directive(
        'downgradedComponent',
        downgradeComponent({component: AngularComponent}),
    )

Use this in your component template to wait for LazyLoadedAngularModule to be loaded before showing the downgraded component

<div module-lazy-load="LazyLoadedAngularModule">
      <downgraded-component></downgraded-component>
</div>

Note I modified this code on the fly in github, copy/paste may not work straight away

Good luck

@blemaire
Copy link

blemaire commented Dec 6, 2021

I have an event simpler solution if anybody ever needs to do this again ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: upgrade Issues related to AngularJS → Angular upgrade APIs feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature freq1: low P4 A relatively minor issue that is not relevant to core functions workaround2: non-obvious
Projects
None yet
Development

No branches or pull requests