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

Add parentInjector to Platform Factories #43198

Open
DavidANeil opened this issue Aug 18, 2021 · 4 comments
Open

Add parentInjector to Platform Factories #43198

DavidANeil opened this issue Aug 18, 2021 · 4 comments
Assignees
Labels
area: core Issues related to the framework runtime core: di feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature
Milestone

Comments

@DavidANeil
Copy link
Contributor

Which @angular/* package(s) are relevant/releated to the feature request?

platform-browser-dynamic, platform-browser, platform-server

Description

We would like to extend the functionality of the existing platform factories by adding a parent injector to the platform. The only way to do this currently involves copying providers from Angular source code, some of which are private exports, and some which have to be massaged in order to work at all.

This would allow us to change the code

export function _document() {
    ɵsetDocument(document);
    return document;
}
export function OurPlatformDynamic(ourInjector: OurInjector): PlatformRef {
    return createPlatform(
        Injector.create({
            providers: [
                {provide: PlatformRef, deps: [Injector]},
                {provide: TestabilityRegistry, deps: []},
                {provide: ɵConsole, deps: []},
                {provide: COMPILER_OPTIONS, useValue: {}, multi: true},
                {provide: CompilerFactory, useClass: JitCompilerFactory, deps: [COMPILER_OPTIONS]},
                {provide: PLATFORM_ID, useValue: 'browser'},
                {provide: PLATFORM_INITIALIZER, useValue: ɵinitDomAdapter, multi: true},
                {provide: DOCUMENT, useFactory: _document, deps: []},
                {
                    provide: COMPILER_OPTIONS,
                    useValue: {providers: [{provide: ResourceLoader, useClass: ɵResourceLoaderImpl, deps: []}]},
                    multi: true,
                },
            ],
            parent: ourInjector.angularInjectorApi,
            name: 'Platform: Bridge',
        }),
    );
}

to the much terser:

export function OurPlatformDynamic(ourInjector: OurInjector): PlatformRef {
    return platformBrowserDynamic([], ourInjector.angularInjectorApi);
}

Proposed solution

Change the return type of createPlatformFactory to (extraProviders?: StaticProvider[], parentInjector?: Injector) => PlatformRef
The implementation would then simply pass parentInjector in when calling Injector.create.

Alternatives considered

Calling createPlatform directly is possible, but then it requires copying the list of built-in providers from the existing platforms. Such as copying _CORE_PLATFORM_PROVIDERS. This is fragile as this would break anytime that list changes.

@pkozlowski-opensource pkozlowski-opensource added area: core Issues related to the framework runtime core: di labels Aug 18, 2021
@ngbot ngbot bot modified the milestone: needsTriage Aug 18, 2021
@flash-me
Copy link
Contributor

flash-me commented Aug 19, 2021

Yes. Yes. Yes! This would help in several use cases. There is no way to share something between several bootstrapped apps that share the same platform.

My approach currently is different, but also very unhandy! Allowing to provide own Injector or even additional Providers except StaticProvider would be very helpful.

My Approach:

// An NgModule that provides everything I want to share at platform level
@NgModule({
  providers: [
    // Add here custom providers
  ]
})
export class CustomPlatfromProviderModule { }

/**
 * Returns the CustomPlatfromProviderModule NgModuleRef
 * Bootstrap, if necessary
 */
export const getPlatformModule = (function () {
  // Closure holdung the singleton module ref
  let platformModule: NgModuleRef<CustomPlatfromProviderModule>;
  return () =>
    new Promise<NgModuleRef<CustomPlatfromProviderModule>>(
      resolve =>
        platformModule
          ? resolve(platformModule)
          : platformBrowser()
            .bootstrapModule(CustomPlatfromProviderModule)
            .then(moduleRef => {
              platformModule ||= moduleRef;
              resolve(platformModule);
            })
    );
})();

/**
 * Bootstraps an NgModule with the custom Platform module
 * @param ngModule
 */
export const customBootstrap = <T>(ngModule: Type<T>): Promise<NgModuleRef<T>> =>
  getPlatformModule()
    .then(pf =>
      (getModuleFactory(ngModule.name) as NgModuleFactory<T>).create(pf.injector))
    .catch((error: Error) => console.error(`Error trying to bootstrap mfe ${ngModule}\n${error?.message}`, error) as any)

But This approach requires to provide NgModule with an id

@NgModule({
  id: ExampleModule.name // <-- required for getModuleFactory to work
})
export class ExampleModule { }

But finally, one would able to bootstrap with custom injector / providers

customBootstrap(ExampleModule)

But all in all, very unhandy.

cheers
flash ⚡

@alxhub alxhub added the feature Issue that requests a new feature label Aug 20, 2021
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Aug 20, 2021
@angular-robot
Copy link
Contributor

angular-robot bot commented Aug 21, 2021

This feature request is now candidate for our backlog! In the next phase, the community has 60 days to upvote. If the request receives more than 20 upvotes, we'll move it to our consideration list.

You can find more details about the feature request process in our documentation.

@angular-robot angular-robot bot added feature: votes required Feature request which is currently still in the voting phase feature: under consideration Feature request for which voting has completed and the request is now under consideration and removed feature: votes required Feature request which is currently still in the voting phase labels Aug 21, 2021
@AndrewKushnir AndrewKushnir added this to Inbox in Feature Requests via automation Sep 9, 2021
@AndrewKushnir AndrewKushnir moved this from Inbox to Needs Discussion in Feature Requests Sep 9, 2021
@alxhub alxhub self-assigned this Sep 9, 2021
@alxhub alxhub moved this from Needs Discussion to Waiting on Followup in Feature Requests Sep 9, 2021
@alxhub
Copy link
Member

alxhub commented Feb 4, 2022

The "platform" in Angular is intended to be the top level of the DI hierarchy. It's possible to extend the platform injector in several ways:

  1. adding providers by creating your own platform factory via createPlatformFactory
  2. adding providers when instantiating the platform (platformBrowser([{provide: ...}]))
  3. by annotating a service with @Injectable({providedIn: 'platform'})

These options allow for a lot of extensibility of the platform concept, without needing an option for a parent injector. Are there use cases which can't be satisfied by one of the above approaches?

@DavidANeil
Copy link
Contributor Author

It is a matter of tying Angular to code that exists outside of the Angular ecosystem.
We have an injector that does not depend on Angular, which is the backbone of a large portion of our existing codebase.
What we previously did, was, when bootstrapping the application, we would have our injector produce a list of providers that can be consumed by the Angular platform:

export function bridgeToAngular(lucidInjector: LucidInjector) {
    const angularProviders = lucidInjector.providers.map((injectionKey) => {
        return {
            provide: injectionKey,
            useFactory: () => {
                return lucidInjector.get(injectionKey);
            },
        };
    });
    return [{provide: LucidInjector, useValue: lucidInjector}, angularProviders];
}

This worked well for many years (2017-2021), but we ran into issues when we started adding the ability to add new LucidInjectables when new code was lazily loaded.
These worked fine for our injector, but the Angular injector wasn't aware of them, because they are not annotated with @Injectable, and they are not in the platform injector's providers.

We thought of trying to re-bridge everything in the lazily loaded NgModules, but then classes annotated with @Injectable({providedIn: 'root'}) that were also loaded lazily were unable to inject our loaded injectables because we could only add providers at the NgModule level, which was a child, not parent, of the root injector they were using.

Which led us to the solution of providing our injector (well, a version of it that has an Angular-compatible API), as the parent of the platform's injector.

Since createPlatform supports passing in an injector, we were able to do this simply enough, unfortunately we wanted to preserve the functionality of the existing platform factories platformBrowserDynamic and platformBrowser. Which meant copying their (unfortunately private) provider lists to our application, which is what you see in the first code block of the original post.

tl;dr: We needed a way to augment the providers of the root or platform injectors after construction, and a parent injector that we control was the easiest way to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to the framework runtime core: di feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature
Projects
No open projects
Feature Requests
Waiting on Followup
Development

Successfully merging a pull request may close this issue.

5 participants