Skip to content

Conversation

@AleksanderBodurri
Copy link
Member

@AleksanderBodurri AleksanderBodurri commented Jan 4, 2023

See individual commits

@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Jan 4, 2023
@ngbot ngbot bot added this to the Backlog milestone Jan 19, 2023
@AleksanderBodurri AleksanderBodurri force-pushed the track-injected-dependencies branch 6 times, most recently from 7ae67df to 8309909 Compare May 22, 2023 20:54
@AleksanderBodurri AleksanderBodurri marked this pull request as ready for review May 22, 2023 20:56
@AleksanderBodurri AleksanderBodurri force-pushed the track-injected-dependencies branch from 8309909 to 9aa4ed2 Compare May 22, 2023 20:59
@AleksanderBodurri AleksanderBodurri force-pushed the track-injected-dependencies branch from 9aa4ed2 to 358bfeb Compare May 22, 2023 21:12
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AleksanderBodurri great work! 👍

A couple high-level comments:

  • it'd be helpful to add more comments into the code to cover function contracts as well as some tricky parts of the logic
  • it'd be really awesome to minimize the footprint of the debug-related code in the main/production code, so it's easier to follow and see where dev mode starts and ends
  • some functions would benefit from refactoring to split them into smaller fns

Thank you.

@AleksanderBodurri AleksanderBodurri force-pushed the track-injected-dependencies branch 3 times, most recently from 2b37766 to 9194965 Compare May 24, 2023 01:26
@AleksanderBodurri AleksanderBodurri force-pushed the track-injected-dependencies branch 11 times, most recently from 11a7675 to 2e5205e Compare May 30, 2023 15:41
@AndrewKushnir AndrewKushnir added this to the v16.2 candidates milestone Jul 25, 2023
@AleksanderBodurri AleksanderBodurri force-pushed the track-injected-dependencies branch 2 times, most recently from 94f339e to b5512fe Compare July 28, 2023 07:09
Currently, understanding dependency injection in Angular requires a lot of context and has been sited in our surveys as one of the largest points of confusion that our users have with the framework. This commit is the beginning of our approach to make debugging dependency injection in Angular easier.

This commit introduces injector profiler callbacks in parts of the framework to emit injector events. It also introduces a default handler for these events. This default handler parses the stream of events to construct some data structures that will support new injector debug APIs.

We have implemented a similar pattern in the past to minimize overhead. There is also the possiblity of making the internal `setInjectorProfiler` function a public debug API in the future, so that users can implement their own handlers to debug DI events. Lastly DI in Angular maps nicely to a stream of events.

For production applications there is no runtime overhead. For applications in dev mode there is some additional overhead from the default profiler handling injector events and holding debug data in memory.

For production applications, dead code elimination should strip all of the code used by this PR.
…hProviders` to callback style interfaces

walkProviderTree and processInjectorTypesWithProviders both perform some generic traversal logic of the import graph of an input NgModule or Standalone component. Currently, these functions pass around a `providersOut` array that is used to collect providers at each step of the traversal.

This PR converts those functions to accept visitor callbacks instead of the `providersOut` array. This is done to make the traversal logic of these functions reusable, while leaving it up to the visitor to determine the logic that fires for each visited node.

This refactor would allow us to reuse `walkProviderTree` for injector debugging APIs that could support some cool features in DevTools, like tracing the injector resolution path of an injected property on a component instance all the way up to the specific imported module/standalone component.
This commit introduces 3 new APIs.

getDependenciesFromInstantiation:
- Given an injector and a token that was instantiated by that injector, discover all of the things were injected in the instance of that token
- This API is meant to enable recursive inspection of dependencies. Dependencies returned by this API include which injector they were providedIn, which enables the continous use of getDependenciesFromInstantiation to determine the dependencies of dependencies

getInjectorProviders:
- Given an injector, discover all of the providers that were configured in that injector.
- This API returns information on the configured providers of an injector, including the import path that leads to the container that the provider originated from (NgModule or standalone directive). This enables fine grained inspection to determine where a specific provider comes from.

getInjectorParent:
- Given an injector, discover the parent of that injector.
- This function is meant to be used recursively to discover the entire resolution path from a starting injector to the NullInjector.

These APIs were designed to be used together. For example, getInjectorParent can be used to discover the structure of an injector hierarchy. Once that's done, getInjectorProviders can be used to determine the providers of each injector in that hierarchy.

Another example: getDependenciesFromInstantiation can be used to discover the dependencies of a specific injector constructed instance. From there, we can use getInjectorParent to discover the injector resolution path and map each dependency to a path from the starting injector to the injector that it was provided in.
…ng APIs

    Creates unit tests for the following APIs

    - setInjectorProfiler
    - getInjectorProviders
    - getInjectorResolutionPath
    - getDependenciesFromInjectable

    Modifies existing tests in

    - packages/examples/core/di/ts/injector_spec.ts
    - packages/core/test/render3/jit/declare_injectable_spec.ts
    - packages/core/test/render3/jit/declare_factory_spec.ts

    because they setup framework injector context manually.

    Exports setInjectorProfilerContext in packages/core/src/core_private_export.ts in order for use
    in the the modified tests above.
@AleksanderBodurri AleksanderBodurri force-pushed the track-injected-dependencies branch from b5512fe to 94dc0f2 Compare July 28, 2023 07:15
@AndrewKushnir AndrewKushnir requested a review from alxhub July 29, 2023 00:34
@AndrewKushnir
Copy link
Contributor

Global Presubmit.

@AndrewKushnir
Copy link
Contributor

FYI, the Global Presubmit is "green".

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: global presubmit The PR is in need of a google3 global presubmit labels Jul 31, 2023
// these are the only providers that do not go through the value hydration logic
// where this event would normally be emitted from.
if (isValueProvider(provider)) {
emitInstanceCreatedByInjectorEvent(provider.useValue);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Instance created by injector" is a bit confusing here, since we're not creating any instance in the injector. Maybe "dependency satisfied by injector" is a more accurate phrase?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh good point. Lets address this and #48639 (comment) in a followup PR. I'll make a note of it so I don't forget

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still applies :)

@alxhub
Copy link
Member

alxhub commented Aug 1, 2023

This PR was merged into the repository by commit c1dee4c.

@alxhub alxhub closed this in ff4d1b4 Aug 1, 2023
alxhub pushed a commit that referenced this pull request Aug 1, 2023
…hProviders` to callback style interfaces (#48639)

walkProviderTree and processInjectorTypesWithProviders both perform some generic traversal logic of the import graph of an input NgModule or Standalone component. Currently, these functions pass around a `providersOut` array that is used to collect providers at each step of the traversal.

This PR converts those functions to accept visitor callbacks instead of the `providersOut` array. This is done to make the traversal logic of these functions reusable, while leaving it up to the visitor to determine the logic that fires for each visited node.

This refactor would allow us to reuse `walkProviderTree` for injector debugging APIs that could support some cool features in DevTools, like tracing the injector resolution path of an injected property on a component instance all the way up to the specific imported module/standalone component.

PR Close #48639
alxhub pushed a commit that referenced this pull request Aug 1, 2023
This commit introduces 3 new APIs.

getDependenciesFromInstantiation:
- Given an injector and a token that was instantiated by that injector, discover all of the things were injected in the instance of that token
- This API is meant to enable recursive inspection of dependencies. Dependencies returned by this API include which injector they were providedIn, which enables the continous use of getDependenciesFromInstantiation to determine the dependencies of dependencies

getInjectorProviders:
- Given an injector, discover all of the providers that were configured in that injector.
- This API returns information on the configured providers of an injector, including the import path that leads to the container that the provider originated from (NgModule or standalone directive). This enables fine grained inspection to determine where a specific provider comes from.

getInjectorParent:
- Given an injector, discover the parent of that injector.
- This function is meant to be used recursively to discover the entire resolution path from a starting injector to the NullInjector.

These APIs were designed to be used together. For example, getInjectorParent can be used to discover the structure of an injector hierarchy. Once that's done, getInjectorProviders can be used to determine the providers of each injector in that hierarchy.

Another example: getDependenciesFromInstantiation can be used to discover the dependencies of a specific injector constructed instance. From there, we can use getInjectorParent to discover the injector resolution path and map each dependency to a path from the starting injector to the injector that it was provided in.

PR Close #48639
alxhub pushed a commit that referenced this pull request Aug 1, 2023
…ng APIs (#48639)

    Creates unit tests for the following APIs

    - setInjectorProfiler
    - getInjectorProviders
    - getInjectorResolutionPath
    - getDependenciesFromInjectable

    Modifies existing tests in

    - packages/examples/core/di/ts/injector_spec.ts
    - packages/core/test/render3/jit/declare_injectable_spec.ts
    - packages/core/test/render3/jit/declare_factory_spec.ts

    because they setup framework injector context manually.

    Exports setInjectorProfilerContext in packages/core/src/core_private_export.ts in order for use
    in the the modified tests above.

PR Close #48639
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 1, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…8639)

Currently, understanding dependency injection in Angular requires a lot of context and has been sited in our surveys as one of the largest points of confusion that our users have with the framework. This commit is the beginning of our approach to make debugging dependency injection in Angular easier.

This commit introduces injector profiler callbacks in parts of the framework to emit injector events. It also introduces a default handler for these events. This default handler parses the stream of events to construct some data structures that will support new injector debug APIs.

We have implemented a similar pattern in the past to minimize overhead. There is also the possiblity of making the internal `setInjectorProfiler` function a public debug API in the future, so that users can implement their own handlers to debug DI events. Lastly DI in Angular maps nicely to a stream of events.

For production applications there is no runtime overhead. For applications in dev mode there is some additional overhead from the default profiler handling injector events and holding debug data in memory.

For production applications, dead code elimination should strip all of the code used by this PR.

PR Close angular#48639
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…hProviders` to callback style interfaces (angular#48639)

walkProviderTree and processInjectorTypesWithProviders both perform some generic traversal logic of the import graph of an input NgModule or Standalone component. Currently, these functions pass around a `providersOut` array that is used to collect providers at each step of the traversal.

This PR converts those functions to accept visitor callbacks instead of the `providersOut` array. This is done to make the traversal logic of these functions reusable, while leaving it up to the visitor to determine the logic that fires for each visited node.

This refactor would allow us to reuse `walkProviderTree` for injector debugging APIs that could support some cool features in DevTools, like tracing the injector resolution path of an injected property on a component instance all the way up to the specific imported module/standalone component.

PR Close angular#48639
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
This commit introduces 3 new APIs.

getDependenciesFromInstantiation:
- Given an injector and a token that was instantiated by that injector, discover all of the things were injected in the instance of that token
- This API is meant to enable recursive inspection of dependencies. Dependencies returned by this API include which injector they were providedIn, which enables the continous use of getDependenciesFromInstantiation to determine the dependencies of dependencies

getInjectorProviders:
- Given an injector, discover all of the providers that were configured in that injector.
- This API returns information on the configured providers of an injector, including the import path that leads to the container that the provider originated from (NgModule or standalone directive). This enables fine grained inspection to determine where a specific provider comes from.

getInjectorParent:
- Given an injector, discover the parent of that injector.
- This function is meant to be used recursively to discover the entire resolution path from a starting injector to the NullInjector.

These APIs were designed to be used together. For example, getInjectorParent can be used to discover the structure of an injector hierarchy. Once that's done, getInjectorProviders can be used to determine the providers of each injector in that hierarchy.

Another example: getDependenciesFromInstantiation can be used to discover the dependencies of a specific injector constructed instance. From there, we can use getInjectorParent to discover the injector resolution path and map each dependency to a path from the starting injector to the injector that it was provided in.

PR Close angular#48639
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…ng APIs (angular#48639)

    Creates unit tests for the following APIs

    - setInjectorProfiler
    - getInjectorProviders
    - getInjectorResolutionPath
    - getDependenciesFromInjectable

    Modifies existing tests in

    - packages/examples/core/di/ts/injector_spec.ts
    - packages/core/test/render3/jit/declare_injectable_spec.ts
    - packages/core/test/render3/jit/declare_factory_spec.ts

    because they setup framework injector context manually.

    Exports setInjectorProfilerContext in packages/core/src/core_private_export.ts in order for use
    in the the modified tests above.

PR Close angular#48639
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: devtools detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants