feat: ESLint rule - all ngrx "Fail" actions should have `implements ErrorAction` #19042

Platonn · 2024-07-11T15:23:41Z

Goal: ensure that each NgRx Failure Action in our repo has an error property. It's ensured by enforcing each such action to have implements ErrorAction.

Means:

added ESLint rule which marks a class as invalid when it has /Fail/ in its name but is missing implements ErrorAction
- it's coped ONLY to files with a suffix *.action*.ts ( supposed to be files with NgRx actions )

Moreover, the ESLint rule provides an auto-fixer which:

adds missing implements ErrorAction
imports ErrorAction from @sparatcus/core in the current file

Note: to run unit tests of the ESLint rule, execute:

cd tools/eslint-rules
npx nx run test --watch

fixes https://jira.tools.sap/browse/CXSPA-7756

…ror property in actions implementing ErrorAction

…tion errors

… any' - wherever needed. In non-needed places, removed this line.

…D PLACES

…ded after renaming the parameter names from 'error' to 'payload'

…payload.error;` with `public error: any;`

…ready `implements ErrorAction` but didn't have public `error` property

… an action that already `extends LoaderFailAction`

… of github.com:SAP/spartacus into feature/CXSPA-7198--v2

…OKED PLACES

…` in epic branch

… deprecate it

… deprecate it - OVERLOOKED PLACES

… deprecate it - OVERLOOKED PLACES, part 2

…d`. Moreover create new `error` property based on `payload`.

…Fail`

…Type

…r = payload.error`

…ents

…o feature/CXSPA-7198--v2

tools/eslint-rules/rules/no-ngrx-fail-action-without-error-action-implementation.spec.ts

tools/eslint-rules/rules/utils/import-utils.spec.ts

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

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

Platonn · 2024-07-16T17:09:04Z

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

    this.meta = entityFailMeta(entityType, id, error);
+    this.error = error;


[note]
Here it's OK, to implement this line. Thanks to it, all actions extending this class won't need to implement analogical mapping for this.error property, I guess.

Same for: LoaderFailAction and EntityScopedFailAction

tools/eslint-rules/rules/no-ngrx-fail-action-without-error-action-implementation.spec.ts

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

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

Platonn

✅ LGTM

pawelfras

LGTM too!

Platonn added 30 commits July 4, 2024 13:04

refactor: change error: ErrorActionType to error: any

bf578b2

refactor: change public error: any to public payload: any

12ac5fc

fix compilation errors: 1. rename error to payload, 2. add missing er…

f2b060e

…ror property in actions implementing ErrorAction

refactor: rename 'public error:' to `public payload:' and fix compila…

0ad48ff

…tion errors

refactor: change 'error: any = this.payload.error;' to 'public error:…

50e216c

… any' - wherever needed. In non-needed places, removed this line.

refactor: change error: ErrorActionType to error: any - OVERLOOKE…

b75e409

…D PLACES

fix: rename usages of 'error' to 'payload' in overlooked places - nee…

b4b64d5

…ded after renaming the parameter names from 'error' to 'payload'

refactor: replace `error: ErrorModel | HttpErrorModel | Error = this.…

1f4ba28

…payload.error;` with `public error: any;`

fix: implement missing public error: any property in places that al…

aca7a15

…ready `implements ErrorAction` but didn't have public `error` property

refactor: constructor param public error: any to public payload: any

8bf8862

refactor: remove accidental addition of implements ErrorAction from…

eb38cef

… an action that already `extends LoaderFailAction`

Merge branches 'feature/CXSPA-7198--v2' and 'epic/ssr-error-handling'…

47adeb4

… of github.com:SAP/spartacus into feature/CXSPA-7198--v2

refactor: replace error: ErrorActionType with error: any - OVERLO…

42a5474

…OKED PLACES

refactor: bring back optional error? which was made required `error…

3ca0ab4

…` in epic branch

refactor: bring back optional error? parameter in core actions, but…

d8bb497

… deprecate it

refactor: bring back optional error? parameter in core actions, but…

285cbbb

… deprecate it - OVERLOOKED PLACES

refactor: bring back optional error? parameter in core actions, but…

1e7bafb

… deprecate it - OVERLOOKED PLACES, part 2

eslint fix

703036d

refactor(SetStoreDetailsSuccess): revert name from error to `payloa…

9bfd900

…d`. Moreover create new `error` property based on `payload`.

refactor: revert keyword public from `LoadAnonymousConsentTemplates…

247aab7

…Fail`

revert changes in LoadCmsPageDataFail

71372ac

bring back optional error/payload, but deprecate it

8a2fc22

LoadProductFail: revert swapping scope and payload params

1899bd1

test: revert changes in unit tests from the old PR

8f643d6

fix(EffectsErrorHandlerService): remove leftover usage of ErrorAction…

fd6a173

…Type

test: revert changes in unit tests from the old PR - PART 2

bd3de45

fix places which needed changing this.error = payload to `this.erro…

fb33e65

…r = payload.error`

MVP linter rule

19583eb

add [Spartacus] prefix to eslint rule

8874c94

move matching word Fail to ESLint selector

f4279e5

Platonn and others added 13 commits July 11, 2024 13:56

extract util functions

f040af8

refactor. make things more parametrized

6c3eee1

refactor: tidy up, make params single objects, but not multiple argum…

c26a635

…ents

move utils to separate files

9d6feac

Add license header

bdf14d5

underline only the class name, but not the whole class body

3083f68

Merge branch 'feature/CXSPA-7198--v2' of github.com:SAP/spartacus int…

8423580

…o feature/CXSPA-7198--v2

Merge branch 'epic/ssr-error-handling' into feature/CXSPA-7756

f80c3bd

tidy up PR

664e435

test :add unit tests for the ESLint rule

a359578

chore: add missing tests and fix typo in file name

adf6730

chore: fix eslint issue related to missing interface

ba457bf

fix sonar and static code issues

922ad6b

pawelfras marked this pull request as ready for review July 16, 2024 12:21

pawelfras requested review from a team as code owners July 16, 2024 12:21

Platonn commented Jul 16, 2024

View reviewed changes

tools/eslint-rules/rules/no-ngrx-fail-action-without-error-action-implementation.spec.ts Outdated Show resolved Hide resolved

chore: refactor after review

8b2992c

Platonn commented Jul 17, 2024

View reviewed changes

feature-libs/product-configurator/rulebased/core/state/actions/configurator.action.ts Outdated Show resolved Hide resolved

Platonn commented Jul 17, 2024

View reviewed changes

feature-libs/cart/base/core/store/actions/cart.action.ts Outdated Show resolved Hide resolved

pawelfras added 2 commits July 17, 2024 13:24

refactor after review

4b2e594

chore: remove redundant comment

ff03087

Platonn commented Jul 17, 2024

View reviewed changes

pawelfras approved these changes Jul 17, 2024

View reviewed changes

Trigger Build

00bdec5

pawelfras merged commit 031ec7d into epic/ssr-error-handling Jul 17, 2024
11 checks passed

pawelfras deleted the feature/CXSPA-7756 branch July 17, 2024 13:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ESLint rule - all ngrx "Fail" actions should have `implements ErrorAction` #19042

feat: ESLint rule - all ngrx "Fail" actions should have `implements ErrorAction` #19042

Platonn commented Jul 11, 2024 •

edited

Loading

Platonn Jul 16, 2024

Platonn left a comment

pawelfras left a comment

		this.meta = entityFailMeta(entityType, id, error);
		this.error = error;

feat: ESLint rule - all ngrx "Fail" actions should have implements ErrorAction #19042

feat: ESLint rule - all ngrx "Fail" actions should have implements ErrorAction #19042

Conversation

Platonn commented Jul 11, 2024 • edited Loading

Platonn Jul 16, 2024

Choose a reason for hiding this comment

Platonn left a comment

Choose a reason for hiding this comment

pawelfras left a comment

Choose a reason for hiding this comment

feat: ESLint rule - all ngrx "Fail" actions should have `implements ErrorAction` #19042

feat: ESLint rule - all ngrx "Fail" actions should have `implements ErrorAction` #19042

Platonn commented Jul 11, 2024 •

edited

Loading