refactor: Handle breaking changes in NgRx actions #19013

Platonn · 2024-06-27T14:42:49Z

In previous past PR within this Epic branch, we've made breaking changes - changing constructor signatures of ngrx actions from having error: any to error: <something more specific> (i.e. error: ErrorActionType). In this PR, we revert this change - allowing for error: any but at the same time deprecating it. Moreover, the non-deprecated constructor requires the value to be non-null, non-undefined.

fixes https://jira.tools.sap/browse/CXSPA-7198 (it's part 1/2; the part 2/2 - adding implements ErrorAction - will arrive in a separate PR for the breviety of this PR)

QA steps
To conduct the QA steps, please add the code snippets given below to your app.module.ts (please make sure to import the pasted items; note: for auto-adding missing imports in VSCode you might find this trick helpful).

Table of contents:

⚠️ deprecated optional error (just in EntityFailAction)
⚠️ deprecated undefined error
⚠️ deprecated null error
⚠️ deprecated any error
⚠️ deprecated unknown error
✅ valid plain object error
✅ valid empty object error
✅ valid empty string error

⚠️ DEPRECATED OPTIONAL verify it's deprecated to omit the optional error? 3rd argument in the action EntityFailAction (it's the only one example with optional error? argument. the rest of actions in examples below have a required error param).

export const entityLoaderAction_optionalLastArgError = new EntityFailAction(
  'entityType',
  'entityId'
);

(screenshot: )

⚠️ DEPRECATED UNDEFINED - Verify it's deprecated to pass undefined error object to various actions, e.g. that in your VSCode the names of the constructed classes are crossed out:

const errorUndefined = undefined;
export const actionsUsing_errorUndefined = [
  new LoadCdcUserTokenFail({
    initialActionPayload: {
      baseSite: 'testBaseSite',
      idToken: 'testIdToken',
      signatureTimestamp: 'testSignatureTimestamp',
      UID: 'testUID',
      UIDSignature: 'testUIDSignature',
    },
    error: errorUndefined,
  }),
  new CustomerSearchFail(errorUndefined),
  new CartAddEntryFail({
    cartId: 'testCartId',
    error: errorUndefined,
    productCode: 'testProductCode',
    quantity: 1,
    userId: 'testUserId',
    pickupStore: 'testPickupStore',
  }),
  new EditSavedCartFail({
    cartId: 'testCartId',
    error: errorUndefined,
    userId: 'testUserId',
    saveCartName: 'testSaveCartName',
  }),
  new LoadWishListFail({
    cartId: 'testCartId',
    error: errorUndefined,
  }),
  new CancelReplenishmentOrderFail({
    message: 'error',
  }),
  new UnassignB2BUserUserGroupFail({
    error: errorUndefined,
    orgCustomerId: 'testOrgCustomerId',
    userGroupId: 'testUserGroupId',
  }),
  new AssignBudgetFail({
    budgetCode: 'testBudgetCode',
    error: errorUndefined,
  }),
  new LoadApprovalProcessesFail({
    error: errorUndefined,
  }),
  new LoadPermissionTypesFail({
    error: errorUndefined,
  }),
  new LoadUnitOrdersFail(errorUndefined),
  new LoadCmsComponentFail({
    error: errorUndefined,
    pageContext: { id: 'testPageId', type: PageType.CONTENT_PAGE },
    uid: 'testUid',
  }),
  new LoadCmsNavigationItemsFail('nodeId', errorUndefined),
  new LoadProductFail('productCode123', errorUndefined, 'scope'),
  new LoadUserConsentsFail(errorUndefined),
  new EntityFailAction('entityType', 'entityId', errorUndefined),
];

(screenshot: )

⚠️ DEPREACTED NULL

const errorNull = null;
export const actionsUsing_errorNull = [
  new LoadCdcUserTokenFail({
    initialActionPayload: {
      baseSite: 'testBaseSite',
      idToken: 'testIdToken',
      signatureTimestamp: 'testSignatureTimestamp',
      UID: 'testUID',
      UIDSignature: 'testUIDSignature',
    },
    error: errorNull,
  }),
  new CustomerSearchFail(errorNull),
  new CartAddEntryFail({
    cartId: 'testCartId',
    error: errorNull,
    productCode: 'testProductCode',
    quantity: 1,
    userId: 'testUserId',
    pickupStore: 'testPickupStore',
  }),
  new EditSavedCartFail({
    cartId: 'testCartId',
    error: errorNull,
    userId: 'testUserId',
    saveCartName: 'testSaveCartName',
  }),
  new LoadWishListFail({
    cartId: 'testCartId',
    error: errorNull,
  }),
  new CancelReplenishmentOrderFail({
    message: 'error',
  }),
  new UnassignB2BUserUserGroupFail({
    error: errorNull,
    orgCustomerId: 'testOrgCustomerId',
    userGroupId: 'testUserGroupId',
  }),
  new AssignBudgetFail({
    budgetCode: 'testBudgetCode',
    error: errorNull,
  }),
  new LoadApprovalProcessesFail({
    error: errorNull,
  }),
  new LoadPermissionTypesFail({
    error: errorNull,
  }),
  new LoadUnitOrdersFail(errorNull),
  new LoadCmsComponentFail({
    error: errorNull,
    pageContext: { id: 'testPageId', type: PageType.CONTENT_PAGE },
    uid: 'testUid',
  }),
  new LoadCmsNavigationItemsFail('nodeId', errorNull),
  new LoadProductFail('productCode123', errorNull, 'scope'),
  new LoadUserConsentsFail(errorNull),
  new EntityFailAction('entityType', 'entityId', errorNull),
];

(screenshot: )

⚠️ DEPRECATED ANY

const errorAny: any = <any>{ something: 'but casted to any' };
export const actionsUsing_errorAny = [
  new LoadCdcUserTokenFail({
    initialActionPayload: {
      baseSite: 'testBaseSite',
      idToken: 'testIdToken',
      signatureTimestamp: 'testSignatureTimestamp',
      UID: 'testUID',
      UIDSignature: 'testUIDSignature',
    },
    error: errorAny,
  }),
  new CustomerSearchFail(errorAny),
  new CartAddEntryFail({
    cartId: 'testCartId',
    error: errorAny,
    productCode: 'testProductCode',
    quantity: 1,
    userId: 'testUserId',
    pickupStore: 'testPickupStore',
  }),
  new EditSavedCartFail({
    cartId: 'testCartId',
    error: errorAny,
    userId: 'testUserId',
    saveCartName: 'testSaveCartName',
  }),
  new LoadWishListFail({
    cartId: 'testCartId',
    error: errorAny,
  }),
  new CancelReplenishmentOrderFail({
    message: 'error',
  }),
  new UnassignB2BUserUserGroupFail({
    error: errorAny,
    orgCustomerId: 'testOrgCustomerId',
    userGroupId: 'testUserGroupId',
  }),
  new AssignBudgetFail({
    budgetCode: 'testBudgetCode',
    error: errorAny,
  }),
  new LoadApprovalProcessesFail({
    error: errorAny,
  }),
  new LoadPermissionTypesFail({
    error: errorAny,
  }),
  new LoadUnitOrdersFail(errorAny),
  new LoadCmsComponentFail({
    error: errorAny,
    pageContext: { id: 'testPageId', type: PageType.CONTENT_PAGE },
    uid: 'testUid',
  }),
  new LoadCmsNavigationItemsFail('nodeId', errorAny),
  new LoadProductFail('productCode123', errorAny, 'scope'),
  new LoadUserConsentsFail(errorAny),
  new EntityFailAction('entityType', 'entityId', errorAny),
];

(screenshot: )

⚠️ DEPRECATED UNKNOWN

const errorUnknown: unknown = <unknown>{ something: 'but casted to unknown' };
export const actionsUsing_errorUnknown = [
  new LoadCdcUserTokenFail({
    initialActionPayload: {
      baseSite: 'testBaseSite',
      idToken: 'testIdToken',
      signatureTimestamp: 'testSignatureTimestamp',
      UID: 'testUID',
      UIDSignature: 'testUIDSignature',
    },
    error: errorUnknown,
  }),
  new CustomerSearchFail(errorUnknown),
  new CartAddEntryFail({
    cartId: 'testCartId',
    error: errorUnknown,
    productCode: 'testProductCode',
    quantity: 1,
    userId: 'testUserId',
    pickupStore: 'testPickupStore',
  }),
  new EditSavedCartFail({
    cartId: 'testCartId',
    error: errorUnknown,
    userId: 'testUserId',
    saveCartName: 'testSaveCartName',
  }),
  new LoadWishListFail({
    cartId: 'testCartId',
    error: errorUnknown,
  }),
  new CancelReplenishmentOrderFail({
    message: 'error',
  }),
  new UnassignB2BUserUserGroupFail({
    error: errorUnknown,
    orgCustomerId: 'testOrgCustomerId',
    userGroupId: 'testUserGroupId',
  }),
  new AssignBudgetFail({
    budgetCode: 'testBudgetCode',
    error: errorUnknown,
  }),
  new LoadApprovalProcessesFail({
    error: errorUnknown,
  }),
  new LoadPermissionTypesFail({
    error: errorUnknown,
  }),
  new LoadUnitOrdersFail(errorUnknown),
  new LoadCmsComponentFail({
    error: errorUnknown,
    pageContext: { id: 'testPageId', type: PageType.CONTENT_PAGE },
    uid: 'testUid',
  }),
  new LoadCmsNavigationItemsFail('nodeId', errorUnknown),
  new LoadProductFail('productCode123', errorUnknown, 'scope'),
  new LoadUserConsentsFail(errorUnknown),
  new EntityFailAction('entityType', 'entityId', errorUnknown),
];

(screenshot: )

✅ VALID PLAIN OBJECT

const errorPlainObject = { message: 'error message 1' };
export const actionsUsing_errorPlainObject = [
  new LoadCdcUserTokenFail({
    initialActionPayload: {
      baseSite: 'testBaseSite',
      idToken: 'testIdToken',
      signatureTimestamp: 'testSignatureTimestamp',
      UID: 'testUID',
      UIDSignature: 'testUIDSignature',
    },
    error: errorPlainObject,
  }),
  new CustomerSearchFail(errorPlainObject),
  new CartAddEntryFail({
    cartId: 'testCartId',
    error: errorPlainObject,
    productCode: 'testProductCode',
    quantity: 1,
    userId: 'testUserId',
    pickupStore: 'testPickupStore',
  }),
  new EditSavedCartFail({
    cartId: 'testCartId',
    error: errorPlainObject,
    userId: 'testUserId',
    saveCartName: 'testSaveCartName',
  }),
  new LoadWishListFail({
    cartId: 'testCartId',
    error: errorPlainObject,
  }),
  new CancelReplenishmentOrderFail({
    message: 'error',
  }),
  new UnassignB2BUserUserGroupFail({
    error: errorPlainObject,
    orgCustomerId: 'testOrgCustomerId',
    userGroupId: 'testUserGroupId',
  }),
  new AssignBudgetFail({
    budgetCode: 'testBudgetCode',
    error: errorPlainObject,
  }),
  new LoadApprovalProcessesFail({
    error: errorPlainObject,
  }),
  new LoadPermissionTypesFail({
    error: errorPlainObject,
  }),
  new LoadUnitOrdersFail(errorPlainObject),
  new LoadCmsComponentFail({
    error: errorPlainObject,
    pageContext: { id: 'testPageId', type: PageType.CONTENT_PAGE },
    uid: 'testUid',
  }),
  new LoadCmsNavigationItemsFail('nodeId', errorPlainObject),
  new LoadProductFail('productCode123', errorPlainObject, 'scope'),
  new LoadUserConsentsFail(errorPlainObject),
  new EntityFailAction('entityType', 'entityId', errorPlainObject),
];

(screenshot: )

✅ VALID EMPTY OBJECT

const errorEmptyObject = {};
export const actionsUsing_errorEmptyObject = [
  new LoadCdcUserTokenFail({
    initialActionPayload: {
      baseSite: 'testBaseSite',
      idToken: 'testIdToken',
      signatureTimestamp: 'testSignatureTimestamp',
      UID: 'testUID',
      UIDSignature: 'testUIDSignature',
    },
    error: errorEmptyObject,
  }),
  new CustomerSearchFail(errorEmptyObject),
  new CartAddEntryFail({
    cartId: 'testCartId',
    error: errorEmptyObject,
    productCode: 'testProductCode',
    quantity: 1,
    userId: 'testUserId',
    pickupStore: 'testPickupStore',
  }),
  new EditSavedCartFail({
    cartId: 'testCartId',
    error: errorEmptyObject,
    userId: 'testUserId',
    saveCartName: 'testSaveCartName',
  }),
  new LoadWishListFail({
    cartId: 'testCartId',
    error: errorEmptyObject,
  }),
  new CancelReplenishmentOrderFail({
    message: 'error',
  }),
  new UnassignB2BUserUserGroupFail({
    error: errorEmptyObject,
    orgCustomerId: 'testOrgCustomerId',
    userGroupId: 'testUserGroupId',
  }),
  new AssignBudgetFail({
    budgetCode: 'testBudgetCode',
    error: errorEmptyObject,
  }),
  new LoadApprovalProcessesFail({
    error: errorEmptyObject,
  }),
  new LoadPermissionTypesFail({
    error: errorEmptyObject,
  }),
  new LoadUnitOrdersFail(errorEmptyObject),
  new LoadCmsComponentFail({
    error: errorEmptyObject,
    pageContext: { id: 'testPageId', type: PageType.CONTENT_PAGE },
    uid: 'testUid',
  }),
  new LoadCmsNavigationItemsFail('nodeId', errorEmptyObject),
  new LoadProductFail('productCode123', errorEmptyObject, 'scope'),
  new LoadUserConsentsFail(errorEmptyObject),
  new EntityFailAction('entityType', 'entityId', errorEmptyObject),
];

(screenshot: )

✅ VALID EMPTY STRING

const errorEmptyString = '';
export const actionsUsing_errorEmptyString = [
  new LoadCdcUserTokenFail({
    initialActionPayload: {
      baseSite: 'testBaseSite',
      idToken: 'testIdToken',
      signatureTimestamp: 'testSignatureTimestamp',
      UID: 'testUID',
      UIDSignature: 'testUIDSignature',
    },
    error: errorEmptyString,
  }),
  new CustomerSearchFail(errorEmptyString),
  new CartAddEntryFail({
    cartId: 'testCartId',
    error: errorEmptyString,
    productCode: 'testProductCode',
    quantity: 1,
    userId: 'testUserId',
    pickupStore: 'testPickupStore',
  }),
  new EditSavedCartFail({
    cartId: 'testCartId',
    error: errorEmptyString,
    userId: 'testUserId',
    saveCartName: 'testSaveCartName',
  }),
  new LoadWishListFail({
    cartId: 'testCartId',
    error: errorEmptyString,
  }),
  new CancelReplenishmentOrderFail({
    message: 'error',
  }),
  new UnassignB2BUserUserGroupFail({
    error: errorEmptyString,
    orgCustomerId: 'testOrgCustomerId',
    userGroupId: 'testUserGroupId',
  }),
  new AssignBudgetFail({
    budgetCode: 'testBudgetCode',
    error: errorEmptyString,
  }),
  new LoadApprovalProcessesFail({
    error: errorEmptyString,
  }),
  new LoadPermissionTypesFail({
    error: errorEmptyString,
  }),
  new LoadUnitOrdersFail(errorEmptyString),
  new LoadCmsComponentFail({
    error: errorEmptyString,
    pageContext: { id: 'testPageId', type: PageType.CONTENT_PAGE },
    uid: 'testUid',
  }),
  new LoadCmsNavigationItemsFail('nodeId', errorEmptyString),
  new LoadProductFail('productCode123', errorEmptyString, 'scope'),
  new LoadUserConsentsFail(errorEmptyString),
  new EntityFailAction('entityType', 'entityId', errorEmptyString),
];

(screenshot: )

Platonn · 2024-06-27T14:54:48Z

feature-libs/organization/administration/core/store/actions/b2b-user.action.ts

@@ -196,12 +223,20 @@ export class LoadB2BUsers extends StateUtils.EntityLoadAction {
 }

 export class LoadB2BUsersFail extends StateUtils.EntityFailAction {
-  error: ErrorActionType = this.payload.error;


[NOTE]
such lines are not needed in classes that extend from StateUtils.EntityFailAction. It's already done inside StateUtils.EntityFailAction super class

…ase: constructor param `payload: any`, but looking up a property `payload.error` inside

… its JSDoc

…tor deprecation

…ce over deprecated `any`) and change `null | undefined` to `any`

…onstructor" to "for distinguishing deprecated constructor

…." + fix the updated import paths of error-action.ts

Platonn · 2024-06-28T20:22:58Z

feature-libs/cart/base/core/store/actions/cart.action.ts

-interface CreateCartFailPayload extends CreateCartPayload {
-  error: ErrorActionType;
-}
-


[NOTE]

Removed this interface in favor of inline info about error:

CreateCartPayload & { error: any }

CreateCartPayload & { error: ActionErrorProperty }`

Platonn · 2024-06-28T20:26:03Z

feature-libs/cart/wish-list/core/store/actions/wish-list.action.ts

@@ -79,13 +89,28 @@ interface LoadWishListFailPayload {
   * temporary cart used to track loading/error state or to normal wish list entity.
   */
  cartId: string;
-  error: ErrorActionType;
+  error: ActionErrorProperty;


[NOTE]

I didn't split the LoadWishListFailPayload, to keep benefiting from its useful existing JSDocs

Platonn · 2024-06-28T20:34:55Z

feature-libs/pickup-in-store/core/store/actions/pickup-location.action.ts

+interface TypeOf_SetStoreDetailsFailure {
+  (props: { error: ActionErrorProperty }): {
+    error: ActionErrorProperty;
+  } & TypedAction<typeof STORE_DETAILS_FAIL>;
+
+  /**
+   * @deprecated Use the `error` parameter with a non-null, non-undefined value.
+   *             Support for `null` or `undefined` will be removed in future versions,
+   *             along with the feature toggle `ssrStrictErrorHandlingForHttpAndNgrx`.
+   */
+  (props: { error: any }): { error: any } & TypedAction<
+    typeof STORE_DETAILS_FAIL
+  >;
+}
+
+// Note for future: when removing the old deprecated code, let's do the following:
+// - remove `TypeOf_SetStoreDetailsFailure`
+// - replace the implementation of `SetStoreDetailsFailure` with the following:
+// ```
+// export const SetStoreDetailsFailure = createAction(
+//   STORE_DETAILS_FAIL,
+//   props<{ error: ActionErrorProperty }>()
+// );
+// ```
+
+export const SetStoreDetailsFailure: TypeOf_SetStoreDetailsFailure =
+  createAction(STORE_DETAILS_FAIL, props<{ error: any }>());


[NOTE]

It's a single exceptional case where NgRx actions are not classes (with constructors), but factories created with the createAction ngrx util.

This exceptional case required an exceptional solution.

…| undefinied instead of any in deprecated constructor

…+ reorder consctructor delcarations

Platonn · 2024-06-28T20:46:49Z

integration-libs/cdc/core/store/actions/cdc-user-token.action.ts

-interface LoadUserTokenFailurePayload {
-  error: ErrorActionType;
-  initialActionPayload: LoadUserTokenPayload;
-}
-


[NOTE]

Removed in favor of defining initialActionPayload: LoadUserTokenPayload separately, and error: <specific_type> separately

…aram `error`

Platonn · 2024-07-01T09:06:56Z

projects/core/src/model/error-action.ts

[NOTE]

Moved within core lib. From /model/error-action.ts to error-handling/effects-error-handler/error-action.ts

Platonn · 2024-07-01T09:09:25Z

projects/core/src/state/utils/entity-loader/entity-loader.action.ts

+  /**
+   * @deprecated Please use the `error` parameter with a non-null, non-undefined value.
+   *             Support for `null` or `undefined` will be removed in future versions,
+   *             along with the feature toggle `ssrStrictErrorHandlingForHttpAndNgrx`.
+   */
+  constructor(
+    entityType: string,
+    id: EntityId,
+    // eslint-disable-next-line @typescript-eslint/unified-signatures -- needed to deprecate only the old constructor
+    error: any
+  );
+  /**
+   * @deprecated Please use the `error` parameter with a non-null, non-undefined value.
+   *             The `error` parameter will become required in future versions,
+   *             along with removing the feature toggle `ssrStrictErrorHandlingForHttpAndNgrx`.
+   */
+  constructor(entityType: string, id: EntityId);


[NOTE]

In this exceptional case I added 2 (but not 1) deprecated constructors, because:

error can be any

AND MOREOVER error can be not passed at all (as error? argument is optional)

Platonn · 2024-07-01T09:17:06Z

projects/core/src/user/store/actions/billing-countries.action.ts

+  constructor(error: ActionErrorProperty);
+  /**
+   * @deprecated Please use the `error` parameter with a non-null, non-undefined value.
+   *             Support for `null` or `undefined` will be removed in future versions,
+   *             along with the feature toggle `ssrStrictErrorHandlingForHttpAndNgrx`.
+   */
+  constructor(
+    // eslint-disable-next-line @typescript-eslint/unified-signatures -- for distinguishing deprecated constructor
+    error: any
+  );


[NOTE]

Some ngrx actions were not refactored in the past PR https://github.com/SAP/spartacus/pull/17657/files from any to ErrorActionType.

So in this PR I'm refactoring them directly from any to ActionErrorProperty.

Note: I expect in the upcoming weeks in develop there might be more new ngrx actions with error: any that we should refactor analogically, before releasing our "Ssr Error Handling" epic

pawelfras · 2024-07-02T11:37:22Z

Have you considered creating a separate ticket to track deprecations which eventually will be removed and attaching its ID to all related places?

pawelfras · 2024-07-02T11:57:08Z

feature-libs/pickup-in-store/core/store/actions/pickup-location.action.ts

-  STORE_DETAILS_FAIL,
-  props<{ error: ErrorActionType }>()
-);
+interface TypeOf_SetStoreDetailsFailure {


query: What about simply calling it SetStoreDetailsFailure? Using TypeOf_ feels strange to me - not sure whether it's a common approach and I'm not familiar with it, but I'm happy to learn. Besides, it is possible to have the same name for const and interface/type. Interface won't be exported outside this file, so I guess it's not a problem to use the same name.

Good point, TypeOf_ it looks odd, even to me. (... "odd even" 🙃).
It would be too bad if anyone gets inspired by this and copies it to other places.

I'll try your suggestion.

pawelfras · 2024-07-02T14:05:24Z

feature-libs/product-configurator/rulebased/core/state/actions/configurator-cart.action.ts

+   *             Support for `null` or `undefined` will be removed in future versions,
+   *             along with the feature toggle `ssrStrictErrorHandlingForHttpAndNgrx`.
+   */
+  constructor(payload: { ownerKey: string; error: null | undefined });


query: Is the order od constructor's overriding proper? Shouldn't the new, not deprecated definition, be first?

Good catch! will fix it

pawelfras · 2024-07-02T14:05:52Z

feature-libs/product-configurator/rulebased/core/state/actions/configurator-variant.action.ts

+   *             Support for `null` or `undefined` will be removed in future versions,
+   *             along with the feature toggle `ssrStrictErrorHandlingForHttpAndNgrx`.
+   */
+  constructor(payload: { ownerKey: string; error: null | undefined });


query: Is the order od constructor's overriding proper? Shouldn't the new, not deprecated definition, be first?

Many thanks for a careful review. Good catch!

Hmm. it seems like I could not have saved and commited latest changes in this file.
Moreover, I can see null | undefined being used here, instead of any (but should be any).

pawelfras · 2024-07-02T14:10:45Z

feature-libs/product-configurator/rulebased/core/state/actions/configurator-variant.action.ts

+   */
+  constructor(payload: { ownerKey: string; error: null | undefined });
+  constructor(
+    // eslint-disable-next-line @typescript-eslint/unified-signatures -- needed to deprecate only the old constructor


query: Shouldn't the eslint-disable-next-line comment be used in a deprecated constructor? Or actually it doesn't matter? Nevertheless, it's different than in other places.

it's a natural consequence of the mistake you commented on there. it fill be fixed by fixing the mistake there

pawelfras · 2024-07-02T14:11:03Z

feature-libs/product-configurator/rulebased/core/state/actions/configurator-cart.action.ts

+   */
+  constructor(payload: { ownerKey: string; error: null | undefined });
+  constructor(
+    // eslint-disable-next-line @typescript-eslint/unified-signatures -- needed to deprecate only the old constructor


query: Shouldn't the eslint-disable-next-line comment be used in a deprecated constructor? Or actually it doesn't matter? Nevertheless, it's different than in other places.

pawelfras · 2024-07-02T14:12:49Z

feature-libs/product-configurator/rulebased/core/state/actions/configurator.action.ts

+   */
+  constructor(payload: { ownerKey: string; error: null | undefined });
+  constructor(
+    // eslint-disable-next-line @typescript-eslint/unified-signatures -- needed to deprecate only the old constructor


query: Shouldn't the eslint-disable-next-line comment be used in a deprecated constructor? Or actually it doesn't matter? Nevertheless, it's different than in other places.

pawelfras · 2024-07-02T14:13:14Z

feature-libs/product-configurator/rulebased/core/state/actions/configurator.action.ts

+   *             Support for `null` or `undefined` will be removed in future versions,
+   *             along with the feature toggle `ssrStrictErrorHandlingForHttpAndNgrx`.
+   */
+  constructor(payload: { ownerKey: string; error: null | undefined });


query: Is the order od constructor's overriding proper? Shouldn't the new, not deprecated definition, be first?

pawelfras · 2024-07-02T14:19:32Z

projects/core/src/state/utils/scoped-loader/entity-scoped-loader.actions.ts

+      error: null | undefined,
+      scope?: string
+    );
+    constructor(


query: Is the order od constructor's overriding proper? Shouldn't the new, not deprecated definition, be first?

pawelfras · 2024-07-02T14:19:52Z

projects/core/src/state/utils/scoped-loader/entity-scoped-loader.actions.ts

+     *             Note: Allowing for `null` or `undefined` will be removed in future versions
+     *             together with the feature toggle `ssrStrictErrorHandlingForHttpAndNgrx`.
+     **/
+    constructor(


query: Shouldn't the eslint-disable-next-line comment be used in a deprecated constructor? Or actually it doesn't matter? Nevertheless, it's different than in other places.

pawelfras · 2024-07-03T08:42:48Z

During QA, I faced an error:

Probably due to missing unknown type in DeprecatedLoadWishListFailPayload

interface DeprecatedLoadWishListFailPayload
  extends Omit<LoadWishListFailPayload, 'error'> {
  error: null | undefined;
}

…lure

@deprecated

- put non-deprecated constructor before the deprecated one - update outdated @deprecate JSdoc - to be more concise - change `null | undefined` to `any` in @deprecated constructors

…to feature/CXSPA-7198

…purposes)

pawelfras

LGTM 👌

…t in favor of the new property `error`

… in favor of the new property `error`

Platonn · 2024-07-09T13:20:22Z

After re-thinking, the value in runtime can be anything unknown, so there is no sense in deprecating null | undefined, because it can be null or undefined anyway in runtime.

So closing in favor of #19036

Platonn added 2 commits June 27, 2024 16:36

first snapshot

d7fcc1c

doc: change eslint-ignore comments

47a725b

Platonn commented Jun 27, 2024

View reviewed changes

Platonn added 12 commits June 27, 2024 16:59

fix(LoadOrgUnitNodesFail): fix deprecated signature for exceptional c…

b932810

…ase: constructor param `payload: any`, but looking up a property `payload.error` inside

refactor(LoadProductFail): rename argument payload to error + Fix…

5168a80

… its JSDoc

fix(LoadPermissionTypesFail): fix non-deprecated constructor

fe7677b

fix(SetStoreDetailsFailure): add deprecated type

7e489dd

fix(ActionErrorProperty): add "Record<any,unknown>" to match construc…

83bff49

…tor deprecation

fix: move non-deprecated constructor to first place (to take preceden…

057837b

…ce over deprecated `any`) and change `null | undefined` to `any`

fix(JSDocs): remove "!!" from JSDocs

3577fbc

fix(@deprecated JSDocs): replace "for distinguishing non-deprecated c…

dbd7dba

…onstructor" to "for distinguishing deprecated constructor

fix: "Entry point @spartacus/core has a circular dependency on itself…

40b74a6

…." + fix the updated import paths of error-action.ts

fix(LoadCdcUserTokenFail): unused interface

6b334df

doc: rephrase @deprecated JSDoc

040a555

doc(@deprecated): add "Please ..."

5768664

Platonn commented Jun 28, 2024

View reviewed changes

Platonn added 2 commits June 28, 2024 22:37

fix(UpdatePriceSummaryFail): constructors in wrong order, using null …

58c3a7d

…| undefinied instead of any in deprecated constructor

fix: change "null | undefined" to "any" in @deprecated consctructors …

61e66d0

…+ reorder consctructor delcarations

Platonn commented Jun 28, 2024

View reviewed changes

fix(EntityFailAction): add 3rd constructor declaration for optional p…

e770c85

…aram `error`

Platonn commented Jul 1, 2024

View reviewed changes

pawelfras reviewed Jul 2, 2024

View reviewed changes

Platonn added 2 commits July 3, 2024 11:02

refactor(TypeOf_SetStoreDetailsFailure): rename to SetStoreDetailsFai…

2195ce2

…lure

fix: align leftover places with the rest of the PR:

e445056

- put non-deprecated constructor before the deprecated one - update outdated @deprecate JSdoc - to be more concise - change `null | undefined` to `any` in @deprecated constructors

Platonn added 2 commits July 3, 2024 11:32

Merge branch 'epic/ssr-error-handling' of github.com:SAP/spartacus in…

70e9aa7

…to feature/CXSPA-7198

fix: remove obsolete const x (added before only for manual testing …

b51b919

…purposes)

pawelfras approved these changes Jul 3, 2024

View reviewed changes

Platonn added 2 commits July 3, 2024 19:18

fix(core tests): bring back public property payload and deprecate i…

9ac6a9c

…t in favor of the new property `error`

fix(asm tests): bring back public property payload and deprecate it…

7bd2709

… in favor of the new property `error`

Platonn closed this Jul 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Handle breaking changes in NgRx actions #19013

refactor: Handle breaking changes in NgRx actions #19013

Platonn commented Jun 27, 2024 •

edited

Loading

Platonn Jun 27, 2024 •

edited

Loading

Platonn Jun 28, 2024

Platonn Jun 28, 2024 •

edited

Loading

Platonn Jun 28, 2024

Platonn Jun 28, 2024 •

edited

Loading

Platonn Jul 1, 2024

Platonn Jul 1, 2024

Platonn Jul 1, 2024

pawelfras commented Jul 2, 2024

pawelfras Jul 2, 2024

Platonn Jul 3, 2024 •

edited

Loading

pawelfras Jul 2, 2024

Platonn Jul 3, 2024

pawelfras Jul 2, 2024

Platonn Jul 3, 2024 •

edited

Loading

pawelfras Jul 2, 2024

Platonn Jul 3, 2024

pawelfras Jul 2, 2024

pawelfras Jul 2, 2024

pawelfras Jul 2, 2024

pawelfras Jul 2, 2024

pawelfras Jul 2, 2024

pawelfras commented Jul 3, 2024

pawelfras left a comment

Platonn commented Jul 9, 2024

refactor: Handle breaking changes in NgRx actions #19013

refactor: Handle breaking changes in NgRx actions #19013

Conversation

Platonn commented Jun 27, 2024 • edited Loading

Platonn Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Platonn Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Platonn Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pawelfras commented Jul 2, 2024

Choose a reason for hiding this comment

Platonn Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Platonn Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pawelfras commented Jul 3, 2024

pawelfras left a comment

Choose a reason for hiding this comment

Platonn commented Jul 9, 2024

Platonn commented Jun 27, 2024 •

edited

Loading

Platonn Jun 27, 2024 •

edited

Loading

Platonn Jun 28, 2024 •

edited

Loading

Platonn Jun 28, 2024 •

edited

Loading

Platonn Jul 3, 2024 •

edited

Loading

Platonn Jul 3, 2024 •

edited

Loading