-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
[RFR] Migrate remaining actions and side effects to Typescript #2824
Conversation
fzaninotto
commented
Jan 28, 2019
•
edited
Loading
edited
- migrate crud Actions
- migrate side effects
- update ra-core global types
Back to WIP, there is a better way to handle identifiers |
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.
👍
and back to RFR |
The data actions are hard to read for non-TypeScript readers. I'd like to move the action creator on top, and the types below, taking advantage of JS hoisting to improve pure JS readability. Something like: import { Record } from '../../types';
import { CREATE } from '../../dataFetchActions';
import { FETCH_END, FETCH_ERROR } from '../fetchActions';
import {
NotificationSideEffect,
RedirectionSideEffect,
} from '../../sideEffect';
export const crudCreate = (
resource: string,
data: any,
basePath: string,
redirectTo: RedirectionSideEffect = 'edit'
): CrudCreateAction => ({
type: CRUD_CREATE,
payload: { data },
meta: {
resource,
fetch: CREATE,
onSuccess: {
notification: {
body: 'ra.notification.created',
level: 'info',
messageArgs: {
smart_count: 1,
},
},
redirectTo,
basePath,
},
onFailure: {
notification: {
body: 'ra.notification.http_error',
level: 'warning',
},
},
},
});
interface RequestPayload {
data: any;
}
export const CRUD_CREATE = 'RA/CRUD_CREATE';
export interface CrudCreateAction {
readonly type: typeof CRUD_CREATE;
readonly payload: RequestPayload;
readonly meta: {
resource: string;
fetch: typeof CREATE;
onSuccess: {
notification: NotificationSideEffect;
redirectTo: RedirectionSideEffect;
basePath: string;
};
onFailure: {
notification: NotificationSideEffect;
};
};
}
export const CRUD_CREATE_LOADING = 'RA/CRUD_CREATE_LOADING';
export interface CrudCreateLoadingAction {
readonly type: typeof CRUD_CREATE_LOADING;
readonly payload: RequestPayload;
readonly meta: {
resource: string;
};
}
export const CRUD_CREATE_FAILURE = 'RA/CRUD_CREATE_FAILURE';
export interface CrudCreateFailureAction {
readonly type: typeof CRUD_CREATE_FAILURE;
readonly error: string | object;
readonly payload: string;
readonly requestPayload: RequestPayload;
readonly meta: {
resource: string;
notification: NotificationSideEffect;
fetchResponse: typeof CREATE;
fetchStatus: typeof FETCH_ERROR;
};
}
export const CRUD_CREATE_SUCCESS = 'RA/CRUD_CREATE_SUCCESS';
export interface CrudCreateSuccessAction {
readonly type: typeof CRUD_CREATE_SUCCESS;
readonly payload: {
data: Record;
};
readonly requestPayload: RequestPayload;
readonly meta: {
resource: string;
notification: NotificationSideEffect;
redirectTo: RedirectionSideEffect;
basePath: string;
fetchResponse: typeof CREATE;
fetchStatus: typeof FETCH_END;
};
} What do you think? |
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.
Awesome! I think our current definition of RedirectionSideEffect
is incomplete though as it also accept a function.