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

[DO NOT MERGE] CXSPA-6939 (PoC) Handling breaking changes related to SSR Error Handling #18789

Open
wants to merge 10 commits into
base: epic/ssr-error-handling
Choose a base branch
from

Conversation

pawelfras
Copy link
Contributor

This PR contains PoC on how to handle breaking changes introduced by features required for SSR Error Handling:

  • ngrx error handling
  • HTTP error handling
  • contract between Angular and ExpressJS
  • changes in rendering cache

@pawelfras pawelfras requested review from a team as code owners May 2, 2024 14:23
core-libs/setup/ssr/optimized-engine/rendering-cache.ts Outdated Show resolved Hide resolved
core-libs/setup/ssr/providers/ssr-providers.ts Outdated Show resolved Hide resolved
core-libs/setup/ssr/providers/ssr-providers.ts Outdated Show resolved Hide resolved
projects/core/src/error-handling/error-handling.module.ts Outdated Show resolved Hide resolved
@@ -24,6 +23,7 @@ import { StateModule } from './state/state.module';

@NgModule({
imports: [
// breaking change - HttpErrorHandlerInterceptor may have featureService inside manipulate the behavior (implemented) OR we can inform in docs that this module has to be imported
Copy link
Contributor

Choose a reason for hiding this comment

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

FeatureToggles for the win. It's easy, simple, documented.
Importing extra modules by customers is an extra overhead. It should be required only in cases when bundlesize of importing this module by default would be considerable.

Comment on lines +35 to +37
// possible ways of handling breaking changes in ALL actions:
// - introduce CustomerSearchFailV2 and deprecate old class
// - overload the constructor of CustomerSearchFail and handle the change there together with deprecating he old one
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry. I don't get the problem - what a breaking change we have? TS Type? Behavior?

And how we're mitigating it?

And if introducing any temporary solutions (e.g. deprecated things), when will we remove them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we discussed, usage of a new type in the constructor or making the constructor's params required provide breaking changes.
Action points for such cases:

  • revert usage of new ErrorActionType type
  • overload the constructor and deprecate the deprecate version with the optional error/payload param
  • create a fallback if the error passed to the constructor is undefined (assign new Error)

core-libs/setup/ssr/providers/ssr-providers.ts Outdated Show resolved Hide resolved
@pawelfras pawelfras force-pushed the feat/CXSPA-6939-poc-handling-breaking-changes branch from 80b3e47 to 8e05980 Compare May 9, 2024 11:58
@pawelfras pawelfras changed the title feat: CXSPA-6939 (PoC) Handling breaking changes related to SSR Error Handling [DO NOT MERGE] CXSPA-6939 (PoC) Handling breaking changes related to SSR Error Handling May 9, 2024
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

Successfully merging this pull request may close these issues.

None yet

2 participants