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] fix placeholder guards #1706

Closed
wants to merge 8 commits into from

Conversation

spike1292
Copy link

When we upgraded to the angular 15 version we experienced the following issues:

  1. Injectable placeholder guards where missing there this context
  2. Guards could not navigate to a route anymore

Description / Motivation

  • Fixed problem 1. With changing guard.canActivate to guard.canActivate.bind(guard)
  • Fixed problem 2. With a custom error JssCanActivateError
  • Added a integration test to make sure the next upgrade does not break lazy loading, guards and routing features.
  • Changed behaviour with lazy loaded placeholder components. Instead of throwing a uncatchable error we now expose the error through an event and a console.warn
    • if needed can be an other pr
    • this enables to gracefully handle an placeholder error. We want to be able to show an nice error to the customer instead of a empty placeholder.
  • Add a lazy load helper to reduce boilerplate for lazy loaded components in modules.

Testing Details

  • Unit Test Added
  • Manual Test/Other
    • created a build, ran npm pack, included tar in our application. saw previous behaviour restored

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)

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

illiakovalenko commented Feb 21, 2024

@spike1292 Thank you for your valuable contribution. Your changes have been incorporated (pulled) into a separate Pull Request to allow for additional modifications before merging. We greatly appreciate your efforts!
I close this PR

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.

2 participants