Skip to content

Commit

Permalink
refactor: review and refactor... (#206)
Browse files Browse the repository at this point in the history
  • Loading branch information
alimd authored Sep 26, 2024
2 parents 145d66e + 5d0ca73 commit 2541101
Show file tree
Hide file tree
Showing 12 changed files with 82 additions and 72 deletions.
19 changes: 14 additions & 5 deletions packages/fetch-state-machine/src/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type {} from '@alwatr/nano-build';
definePackage('@alwatr/fetch-state-machine', __package_version__);

export type ServerRequestState = 'initial' | 'loading' | 'failed' | 'complete';
export type ServerRequestEvent = 'request' | 'requestFailed' | 'requestSuccess';
export type ServerRequestEvent = 'request' | 'requestFailed' | 'requestSucceeded';

export type {FetchOptions};

Expand All @@ -29,7 +29,7 @@ export abstract class AlwatrFetchStateMachineBase<
},
loading: {
requestFailed: 'failed',
requestSuccess: 'complete',
requestSucceeded: 'complete',
},
failed: {
request: 'loading',
Expand All @@ -40,7 +40,7 @@ export abstract class AlwatrFetchStateMachineBase<
} as StateRecord<ServerRequestState | ExtraState, ServerRequestEvent | ExtraEvent>;

protected override actionRecord_ = {
_on_loading_enter: this.requestAction_,
on_loading_enter: this.requestAction_,
} as ActionRecord<ServerRequestState | ExtraState, ServerRequestEvent | ExtraEvent>;

constructor(config: AlwatrFetchStateMachineConfig<ServerRequestState | ExtraState>) {
Expand Down Expand Up @@ -74,13 +74,18 @@ export abstract class AlwatrFetchStateMachineBase<

await this.fetch_(this.currentFetchOptions_);

this.transition_('requestSuccess');
this.requestSucceeded_();
}
catch (error) {
this.requestFailed_(error as Error);
}
}

protected requestSucceeded_(): void {
this.logger_.logMethod?.('requestSucceeded_');
this.transition_('requestSucceeded');
}

protected requestFailed_(error: Error): void {
this.logger_.error('requestFailed_', 'fetch_failed', error);
this.transition_('requestFailed');
Expand All @@ -103,8 +108,12 @@ export abstract class AlwatrFetchStateMachineBase<
}
}

protected override resetToInitialState_(): void {
/**
* Reset the machine to its initial state without notifying, and clean up existing response and state.
*/
protected clean_(): void {
super.resetToInitialState_();
delete this.rawResponse_;
// FIXME: cancel pending fetch
}
}
4 changes: 2 additions & 2 deletions packages/fetch-state-machine/src/fsm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export class AlwatrFetchStateMachine extends AlwatrFetchStateMachineBase {
/**
* Reset the machine to its initial state without notifying, and clean up existing response and state.
*/
reset(): void {
this.resetToInitialState_();
clean(): void {
this.clean_();
}
}
5 changes: 3 additions & 2 deletions packages/fetch-state-machine/src/jfsm-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export abstract class AlwatrJsonFetchStateMachineBase<
}

try {
// TODO: check 304 Not Modified
this.jsonResponse_ = JSON.parse(responseText);
}
catch (err) {
Expand All @@ -30,8 +31,8 @@ export abstract class AlwatrJsonFetchStateMachineBase<
}
}

protected override resetToInitialState_(): void {
super.resetToInitialState_();
protected override clean_(): void {
super.clean_();
delete this.jsonResponse_;
}
}
4 changes: 2 additions & 2 deletions packages/fetch-state-machine/src/jfsm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class AlwatrJsonFetchStateMachine<T extends Json = Json> extends AlwatrJs
/**
* Reset the machine to its initial state without notifying, and clean up existing json response and state.
*/
reset(): void {
this.resetToInitialState_();
clean(): void {
this.clean_();
}
}
6 changes: 3 additions & 3 deletions packages/fsm/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ class MyStateMachine extends FluxStateMachineBase<MyState, MyEvent> {

// Define actions (optional)
this.actionRecord_ = {
'_on_fetch': this.handleFetch,
'_on_success': this.handleSuccess,
'_on_error': this.handleError,
'on_fetch': this.handleFetch,
'on_success': this.handleSuccess,
'on_error': this.handleError,
};
}

Expand Down
2 changes: 1 addition & 1 deletion packages/fsm/demo/light-machine.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class LightMachine extends FiniteStateMachine<State, Event> {
};

this._actionRecord = {
_on_powerLost: this._onPowerLost,
on_powerLost: this._onPowerLost,
};
}

Expand Down
16 changes: 8 additions & 8 deletions packages/fsm/src/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,21 +84,21 @@ export abstract class AlwatrFluxStateMachineBase<S extends string, E extends str
private async postTransition__(eventDetail: StateEventDetail<S, E>): Promise<void> {
this.logger_.logMethodArgs?.('_transitioned', eventDetail);

await this.execAction__(`_on_${eventDetail.event}`, eventDetail);
await this.execAction__(`on_${eventDetail.event}`, eventDetail);

if (eventDetail.from !== eventDetail.to) {
await this.execAction__(`_on_state_exit`, eventDetail);
await this.execAction__(`_on_${eventDetail.from}_exit`, eventDetail);
await this.execAction__(`_on_state_enter`, eventDetail);
await this.execAction__(`_on_${eventDetail.to}_enter`, eventDetail);
await this.execAction__(`on_state_exit`, eventDetail);
await this.execAction__(`on_${eventDetail.from}_exit`, eventDetail);
await this.execAction__(`on_state_enter`, eventDetail);
await this.execAction__(`on_${eventDetail.to}_enter`, eventDetail);
}

if (Object.hasOwn(this, `_on_${eventDetail.from}_${eventDetail.event}`)) {
this.execAction__(`_on_${eventDetail.from}_${eventDetail.event}`, eventDetail);
if (Object.hasOwn(this, `on_${eventDetail.from}_${eventDetail.event}`)) {
this.execAction__(`on_${eventDetail.from}_${eventDetail.event}`, eventDetail);
}
else {
// The action `all_eventName` is executed only if the action `fromState_eventName` is not defined.
this.execAction__(`_on_all_${eventDetail.event}`, eventDetail);
this.execAction__(`on_all_${eventDetail.event}`, eventDetail);
}
}

Expand Down
7 changes: 7 additions & 0 deletions packages/fsm/src/fsm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,11 @@ export abstract class AlwatrFluxStateMachine<S extends string, E extends string>
transition(event: E): void {
this.transition_(event);
}

/**
* Reset machine to initial state without notify.
*/
resetToInitialState(): void {
this.resetToInitialState_();
}
}
14 changes: 7 additions & 7 deletions packages/fsm/src/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ export type StateRecord<S extends string, E extends string> = Partial<Record<S |
export type Action<S extends string, E extends string> = (eventDetail?: StateEventDetail<S, E>) => MaybePromise<void>;

export type ActionName<S extends string, E extends string> =
| `_on_${E}`
| `_on_state_exit`
| `_on_state_enter`
| `_on_${S}_exit`
| `_on_${S}_enter`
| `_on_${S}_${E}`
| `_on_all_${E}`;
| `on_${E}`
| `on_state_exit`
| `on_state_enter`
| `on_${S}_exit`
| `on_${S}_enter`
| `on_${S}_${E}`
| `on_all_${E}`;

export type ActionRecord<S extends string, E extends string> = Partial<Record<ActionName<S, E>, Action<S, E>>>;
27 changes: 11 additions & 16 deletions packages/remote-context/src/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ export abstract class AlwatrRemoteContextStateMachineBase<T extends Json = Json>
offlineCheck: {
requestFailed: 'failed',
cacheNotFound: 'loading',
requestSuccess: 'reloading',
requestSucceeded: 'reloading',
},
/**
* First loading without any cached context.
*/
loading: {
requestFailed: 'failed',
requestSuccess: 'complete',
requestSucceeded: 'complete',
},
/**
* First loading failed without any cached context.
Expand All @@ -52,7 +52,7 @@ export abstract class AlwatrRemoteContextStateMachineBase<T extends Json = Json>
},
reloading: {
requestFailed: 'reloadingFailed',
requestSuccess: 'complete',
requestSucceeded: 'complete',
},
/**
* Reloading failed with previously cached context exist.
Expand All @@ -66,10 +66,10 @@ export abstract class AlwatrRemoteContextStateMachineBase<T extends Json = Json>
};

this.actionRecord_ = {
_on_offlineCheck_enter: this.offlineRequestAction_,
_on_loading_enter: this.onlineRequestAction_,
_on_reloading_enter: this.onlineRequestAction_,
_on_requestSuccess: this.updateContextAction_,
on_offlineCheck_enter: this.offlineRequestAction_,
on_loading_enter: this.onlineRequestAction_,
on_reloading_enter: this.onlineRequestAction_,
on_requestSucceeded: this.updateContextAction_,
};
}

Expand All @@ -93,12 +93,7 @@ export abstract class AlwatrRemoteContextStateMachineBase<T extends Json = Json>
return;
}

if (this.context_ !== undefined && JSON.stringify(this.context_) !== JSON.stringify(this.jsonResponse_)) {
// TODO: improve performance. compare hash value or updatedAt or catch response text.
this.context_ = this.jsonResponse_;
}

this.cleanup_();
this.context_ = this.jsonResponse_;
}

protected override requestFailed_(error: Error): void {
Expand All @@ -112,8 +107,8 @@ export abstract class AlwatrRemoteContextStateMachineBase<T extends Json = Json>
}
}

protected cleanup_(): void {
delete this.rawResponse_;
delete this.jsonResponse_;
protected override clean_(): void {
super.clean_();
delete this.context_;
}
}
7 changes: 7 additions & 0 deletions packages/remote-context/src/remote-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,11 @@ export class AlwatrRemoteContextStateMachine<T extends Json = Json> extends Alwa
request(fetchOptions?: Partial<FetchOptions>): void {
return this.request_(fetchOptions);
}

/**
* Reset the machine to its initial state without notifying, and clean up existing context (include raw response) and state.
*/
clean(): void {
this.clean_();
}
}
43 changes: 17 additions & 26 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2142,16 +2142,7 @@ __metadata:
languageName: node
linkType: hard

"@types/node@npm:*, @types/node@npm:^22.5.2":
version: 22.5.5
resolution: "@types/node@npm:22.5.5"
dependencies:
undici-types: "npm:~6.19.2"
checksum: 10c0/ead9495cfc6b1da5e7025856dcce2591e9bae635357410c0d2dd619fce797d2a1d402887580ca4b336cb78168b195224869967de370a23f61663cf1e4836121c
languageName: node
linkType: hard

"@types/node@npm:^22.7.2":
"@types/node@npm:*, @types/node@npm:^22.5.2, @types/node@npm:^22.7.2":
version: 22.7.2
resolution: "@types/node@npm:22.7.2"
dependencies:
Expand Down Expand Up @@ -2761,16 +2752,16 @@ __metadata:
linkType: hard

"browserslist@npm:^4.22.2":
version: 4.23.3
resolution: "browserslist@npm:4.23.3"
version: 4.24.0
resolution: "browserslist@npm:4.24.0"
dependencies:
caniuse-lite: "npm:^1.0.30001646"
electron-to-chromium: "npm:^1.5.4"
caniuse-lite: "npm:^1.0.30001663"
electron-to-chromium: "npm:^1.5.28"
node-releases: "npm:^2.0.18"
update-browserslist-db: "npm:^1.1.0"
bin:
browserslist: cli.js
checksum: 10c0/3063bfdf812815346447f4796c8f04601bf5d62003374305fd323c2a463e42776475bcc5309264e39bcf9a8605851e53560695991a623be988138b3ff8c66642
checksum: 10c0/95e76ad522753c4c470427f6e3c8a4bb5478ff448841e22b3d3e53f89ecaf17b6984666d6c7e715c370f1e7fa0cf684f42e34e554236a8b2fab38ea76b9e4c52
languageName: node
linkType: hard

Expand Down Expand Up @@ -2851,10 +2842,10 @@ __metadata:
languageName: node
linkType: hard

"caniuse-lite@npm:^1.0.30001646":
version: 1.0.30001662
resolution: "caniuse-lite@npm:1.0.30001662"
checksum: 10c0/4af44610db30b9e63443d984be9d48ab93eef584609b3e87201c10972b9daff0b52674e3ed01f7b7b240460763428a3aa8ef7328d14b76ed31ed464203677007
"caniuse-lite@npm:^1.0.30001663":
version: 1.0.30001664
resolution: "caniuse-lite@npm:1.0.30001664"
checksum: 10c0/db2b431aba41a585191ab1e4d40da0ad349ff32400edac2a167bf6bf92dbf9c704eab03dc60fb89e882ce02478d61c3036b2b1bdce8edf9b2aabda5608bae05e
languageName: node
linkType: hard

Expand Down Expand Up @@ -3451,10 +3442,10 @@ __metadata:
languageName: node
linkType: hard

"electron-to-chromium@npm:^1.5.4":
version: 1.5.27
resolution: "electron-to-chromium@npm:1.5.27"
checksum: 10c0/4a057f469a01829884f2a51f3fc60af7e6a718b15009e4875df122fcdf13babea475ba029af1652a6875b4acfca730c48b13caac5d477d648e622699d3b662bf
"electron-to-chromium@npm:^1.5.28":
version: 1.5.29
resolution: "electron-to-chromium@npm:1.5.29"
checksum: 10c0/ae4849f1fe8d756d30c6f5f992803d8550a98b38a30aecc7d9776858cf229ad05b12cb9f7675f0a89330a077d16e28388cfe394fdd9d0828ffe860c8568c95c2
languageName: node
linkType: hard

Expand Down Expand Up @@ -3786,14 +3777,14 @@ __metadata:
linkType: hard

"eslint-module-utils@npm:^2.8.1, eslint-module-utils@npm:^2.9.0":
version: 2.11.0
resolution: "eslint-module-utils@npm:2.11.0"
version: 2.11.1
resolution: "eslint-module-utils@npm:2.11.1"
dependencies:
debug: "npm:^3.2.7"
peerDependenciesMeta:
eslint:
optional: true
checksum: 10c0/c1b02e83429878ab22596f17a5ac138e51a520e96a5ef89a5a6698769a2d174ab28302d45eb563c0fc418d21a5842e328c37a6e8f294bf2e64e675ba55203dd7
checksum: 10c0/d1c23397eddc42a7824de08348095483bc270a4a3222bc0d54a76382c6411111c33e44a0a1819489e1e209d9e4721de2a8438e7ca4e6fe6be32ff818af9b11b4
languageName: node
linkType: hard

Expand Down

0 comments on commit 2541101

Please sign in to comment.