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

DI: Module provider visibility issue #596

Open
alpharder opened this issue Jul 9, 2024 · 8 comments
Open

DI: Module provider visibility issue #596

alpharder opened this issue Jul 9, 2024 · 8 comments

Comments

@alpharder
Copy link
Contributor

I've created a PR with a failing test case: https://github.com/deepkit/deepkit-framework/pull/595/files#diff-8638469975c860d179cea016ca3ee1f3c2d65fbd6eba97a2096def06beeaa332R193

TL;DR is provider registered within a deeply nested module isn't available for ingestion within the same module, unless this module gets exported to the root.

@marcj
Copy link
Member

marcj commented Jul 10, 2024

The bug in the code here comes from a misunderstanding. Encapsulated services like SessionForRequest can not be set via the root InjectorContext that you have in event.injectorContext. This context operates on the root module (the app module). This module doesn't know anything about encapsulated services like SessionForRequest, so in order to set or get this particular service manually from a Injector or InjectorContext, you need to have a reference to the module it was defined in. Since your listeners is also defined in the same module, you can inject the AppModule it was defined in via dependency injection and then use this reference for the set call:

     class AuthenticationListener {
         @eventDispatcher.listen(httpWorkflow.onAuth)
         onServerMainBootstrap(event: typeof httpWorkflow.onAuth.event, module: AppModule) {
             event.injectorContext.set(
                 SessionForRequest,
                 new SessionForRequest(
                     'sidValue',
                     'uidValue',
                 ),
                 module,
             );
         }
     }

this way the DI container can find the SessionForRequest and set it correctly.

@alpharder
Copy link
Contributor Author

Thanks for the explanation, it makes sense now. I didn't expect event injection context to be root context instead of the listener's module context.

@alpharder
Copy link
Contributor Author

This probably needs to be documented in some way

@alpharder
Copy link
Contributor Author

@marcj Unfortunately, the suggested fix didn't help: https://github.com/deepkit/deepkit-framework/pull/595/files#diff-5a1f8db1e5e23da96c85177fe8129b79e23bf0a80af0053c5407237aa2c1594bR8, result is the same: Controller for route /auth/whoami parameter resolving error: DependenciesUnmetError: Parameter sess is required but provider returned undefined.

@alpharder
Copy link
Contributor Author

Here's the injector that gets built for AuthenticationModule:

'function(token, scope, destination) {\n' +
        '                switch (true) {\n' +
        '                    \n' +
        '            //AuthenticationListener, from AuthenticationModule\n' +
        '            case token === constVar_0: {\n' +
        '                if (injector.instances.i111 !== undefined) return injector.instances.i111;\n' +
        '                CircularDetector.push(constVar_0);\n' +
        '                \n' +
        '                injector.instantiated.i111 = injector.instantiated.i111 ? injector.instantiated.i111 + 1 : 1;\n' +
        '                injector.instances.i111 = new classType_0();\n' +
        '\n' +
        '                \n' +
        '                CircularDetector.pop();\n' +
        '                //no custom provider setup\n' +
        '                return injector.instances.i111;\n' +
        '            }\n' +
        '        \n' +
        '\n' +
        '                    case token === token_0 && scope && scope.name === "http": {\n' +
        '                        return resolveFrom_0.injector.resolver(token_0, scope, destination);\n' +
        '                    }\n' +
        '                    \n' +
        '\n' +
        '            //OAuth2Controller, from AuthenticationModule\n' +
        '            case token === constVar_1 && scope && scope.name === "http": {\n' +
        '                if (scope.instances.i113 !== undefined) return scope.instances.i113;\n' +
        '                CircularDetector.push(constVar_1);\n' +
        '                \n' +
        '                injector.instantiated.i113 = injector.instantiated.i113 ? injector.instantiated.i113 + 1 : 1;\n' +
        '                scope.instances.i113 = new classType_1();\n' +
        '\n' +
        '                \n' +
        '                CircularDetector.pop();\n' +
        '                //no custom provider setup\n' +
        '                return scope.instances.i113;\n' +
        '            }\n' +
        '        \n' +
        '                }\n' +
        '\n' +
        "                tokenNotfoundError(token, 'AuthenticationModule');\n" +
        '            }'

@marcj
Copy link
Member

marcj commented Jul 11, 2024

the fix did help, I tested it locally. I guess you have a new use-case/different code now. the error message indicates that the setter isn't working (maybe injected "module" is wrong), but I haven't looked closely yet.

@alpharder
Copy link
Contributor Author

Please refer to the PR with a failing test case: https://github.com/deepkit/deepkit-framework/pull/595/files

Use case is the same.

Injector wants to resolve value from some other module, because SessionForRequest is exported:

return resolveFrom_0.injector.resolver(token_0, scope, destination);

@marcj
Copy link
Member

marcj commented Nov 12, 2024

@alpharder is this issue still present?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants