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

Fix warning on catchall page due to improper sanitization in <NotFound> component. #4275

Merged
merged 2 commits into from
Jan 13, 2020

Conversation

WiXSL
Copy link
Contributor

@WiXSL WiXSL commented Jan 10, 2020

This could happend from RoutesWithLayout.tsx.

...
<Route
    render={routeProps =>
        createElement(catchAll, {
            ...routeProps,
            title,
        })
    }
 />
...

@fzaninotto
Copy link
Member

No, I don't think we should do that. If a route is to be used as a child of Route, it should expect these props to be passed.

@fzaninotto fzaninotto closed this Jan 13, 2020
@Luwangel
Copy link
Contributor

Luwangel commented Jan 13, 2020

Hi, thanks for your contribution :)

By reading your PR, I'm wondering what issue you try to fix. I think it's the warning on the NotFound page, but you didn't mention it in the description. Could you write what you try to achieve?

Regards,
Adrien

@WiXSL
Copy link
Contributor Author

WiXSL commented Jan 13, 2020

I tried to avoid the staticContext error thrown on NotFound component render.
The comment was to show the origin of the problem.

In the below sample, change the end of the url to something that doesn't exist like .../#/posts2
https://codesandbox.io/s/github/marmelab/react-admin/tree/master/examples/simple

In the console you'll see the error I tried to solve:

Warning: React does not recognize the `staticContext` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `staticcontext` instead. If you accidentally passed it from a parent component, remove it from the DOM element.
    in div (created by NotFound)
    in Authenticated (created by NotFound)
    in NotFound (created by Context.Consumer)
    in Route (created by RoutesWithLayout)
    in Switch (created by RoutesWithLayout)
    in RoutesWithLayout (created by Context.Consumer)

@WiXSL
Copy link
Contributor Author

WiXSL commented Jan 13, 2020

@fzaninotto, The sanitizeRestProps function is called only to sanitize props to be applied to a div element inside NotFound component. Neither staticContext nor match should be applied to a div element.
Exactly the same thing is done in Layout.js already.

const NotFound = ({ className, classes: classesOverride, title, ...rest }) => {
    const classes = useStyles({ classes: classesOverride });
    const translate = useTranslate();
    useAuthenticated();
    return (
        <div
            className={classnames(classes.container, className)}
            {...sanitizeRestProps(rest)} // <-- Here
        >
    //...

@fzaninotto
Copy link
Member

Sorry, I read too fast. Reopening.

@fzaninotto fzaninotto reopened this Jan 13, 2020
@fzaninotto fzaninotto changed the title Sanitize <Route> props passed to <NotFound> component. Fix warning on catchall page due to improper sanitization in <NotFound> component. Jan 13, 2020
@fzaninotto
Copy link
Member

In the future, can you please post a more precise description of the bug you're trying to fix? Otherwise we have to guess and sometimes we guess wrong ;)

@WiXSL
Copy link
Contributor Author

WiXSL commented Jan 13, 2020

No problem, It was my fault, , I assume it was a trivial change to understand.

@fzaninotto fzaninotto merged commit f7ab333 into marmelab:master Jan 13, 2020
@fzaninotto
Copy link
Member

Thanks!

@fzaninotto fzaninotto added this to the 3.1.3 milestone Jan 13, 2020
@WiXSL WiXSL deleted the patch-not-found-error branch January 13, 2020 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants