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

[sitecore-jss-angular] Fixed missing ngModuleRef in lazy loaded components #1717

Closed
wants to merge 1 commit into from

Conversation

osi-oswald
Copy link

Components declared in a lazy loaded Module are no longer able to inject Providers from that Module, they get a NullPointer exception from Angular Injection.

Description / Motivation

When the PlaceholderComponent creates the Rendering Components, it is missing the ngModuleRef (as describen in the Angular documentation for createComponent). Without the ModuleRef the Component falls back to the App Module injector, which does not have the providers defined in the lazy loaded Module.
This change adds back the ngModuleRef when a Rendering Component is being created by the PlaceholderComponent, therefore using the right injector. It used to work in older versions of sitecore-jss-angular and broke with the following commit 4959479. In the older version the componentFactory already included the ngModuleRef, but it's missing in the new componentImplementation.

Testing Details

I built my forked repo locally, integrated it into our project and manually tested it successfully (NullPointer exceptions were no longer thrown for lazy loaded components). Unfortunately I don't have the time or capacity to add new Unit Test's.

  • Unit Test Added
  • Manual Test/Other (Please elaborate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@yavorsk yavorsk added the backlog Issue/PR/discussion is reviewed and added to backlog for the further work label Jan 31, 2024
@yavorsk
Copy link
Contributor

yavorsk commented Jan 31, 2024

Hi @osi-oswald , thanks for your contribution :) we've created a backlog item regarding this pr for review and eventually incorporating it.

@illiakovalenko
Copy link
Contributor

@osi-oswald Hey, I opened PR #1743 and pulled your changes, so I could add more unit tests
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issue/PR/discussion is reviewed and added to backlog for the further work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants