-
Notifications
You must be signed in to change notification settings - Fork 371
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
refactor: two toggles for strict error handling and server error propagation #18815
Conversation
protected isServerErrorPropagationEnabled = | ||
inject(FeatureToggles).serverErrorPropagation; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's:
- inject FeatureConfigService as
private
property - call it inline .isEnabled() in line 36
(it's not to leave unnecessary properties in public API protected
that will need to be removed)
Note: it applies to also other places in this PR
cachingStrategyResolver: (entry) => !entry.error, | ||
ssrErrorHandling: true, | ||
cacheStrategyResolver: (options, entry) => | ||
!!options.cacheErrors && !!entry.error, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- we have to check if
cacheErrors
is expliclity=== true
. Becuase otherwise alsoundefined
makes it falsy by default - and we dont want to toggle it falsy by default. - IMHO semantics of a flag should be falsy when disabled (by default) and truthy when enabled.
What about switching the meaning to avoidCachingErrors
. IMHO it's more attactive and understandable for customers to toggle it on. Moreover when it's falsy (false/undefined) => the new behavior is disabled
/** | ||
* Enable caching of errors. By default, errors are not cached. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let update jsdocs
f8143ee
into
feat/CXSPA-6939-poc-handling-breaking-changes
No description provided.