Skip to content
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

[RFC] Move BasePath meta inside the redirectTo side effect #3254

Closed
fzaninotto opened this issue May 20, 2019 · 1 comment
Closed

[RFC] Move BasePath meta inside the redirectTo side effect #3254

fzaninotto opened this issue May 20, 2019 · 1 comment

Comments

@fzaninotto
Copy link
Member

fzaninotto commented May 20, 2019

The redirectTo side effect, when declared via an action meta, relies on the presence of a basePath field in the meta, too:

export const crudGetOne = (
    resource: string,
    id: Identifier,
    basePath: string,
    refresh: RefreshSideEffect = true
): CrudGetOneAction => ({
    type: CRUD_GET_ONE,
    payload: { id },
    meta: {
        resource,
        fetch: GET_ONE,
        basePath,    // <= this one is only there because of redirectTo below
        onFailure: {
            notification: {
                body: 'ra.notification.item_doesnt_exist',
                level: 'warning',
            },
            redirectTo: 'list',
            refresh,
        },
    },
});

There is no real justification for that, and it doesn't make sense. No other side effect than the redirectTo saga uses that basePath field.

As it adds special cases in the options management, I suggest we move the basePath data where in belongs, in the redirectTo side effect:

export const crudGetOne = (
    resource: string,
    id: Identifier,
    basePath: string,
    refresh: RefreshSideEffect = true
): CrudGetOneAction => ({
    type: CRUD_GET_ONE,
    payload: { id },
    meta: {
        resource,
        fetch: GET_ONE,
-       basePath,
        onFailure: {
            notification: {
                body: 'ra.notification.item_doesnt_exist',
                level: 'warning',
            },
-           redirectTo: 'list',
+           redirect: { to: 'list', basePath },
            refresh,
        },
    },
});

This is a BC for people using actions with custom side effects.

@fzaninotto
Copy link
Member Author

With side effects as hooks (#3425), we no longer need to pass basePath in the metas. But we don't need to remove them either: the change is backwards compatible!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant