sekoyo / universal-react

A universal react starter, with routing, meta, title, and data features
242 stars 50 forks source link

HTTP 200 is sent on page not found #18

Closed keyeh closed 8 years ago

keyeh commented 8 years ago

When a page is not found the response from the server is an HTTP 200 instead of 404.

Example: http://localhost:3000/asdf returns 200 status

Removing <Route path="*" component={NoMatch} /> in Routes.js fixes this, but the NoMatch component doesn't render correctly (it should render as a child of Appcomponent)

sekoyo commented 8 years ago

@keyeh that's a good point - actually the final else I think is never called right? I remember you doing a fix to pass store to it, but I think it's never called?

else {
    handle404(res, store);
}
sekoyo commented 8 years ago

It's possible to check if the route.path === '*' or route.component === NoMatch but it's too dependent on the structure of Routes.js to be a general solution - e.g. https://github.com/DominicTobias/universal-react/pull/19

keyeh commented 8 years ago

@DominicTobias Yes the final else is never called unless you remove the catch all route.

19 Works. Can you explain why this is too dependent on Routes.js structure? As long as there's a * route it should work, right?

Maybe checking the component instead of the route is better.

function routeIsUnmatched(renderProps) {
    return renderProps.components.indexOf(NoMatch) !== -1;
}

Now rendering NoMatch component will always cause a 404.

sekoyo commented 8 years ago

I was wondering if it was possible to nest the * path (probably not), or nest more routes inside of it so that it's not the last one (probably) so indexOf is a good idea 👍

sekoyo commented 8 years ago

Going to go with * as then it's not dependent on a particular component, I don't think you can actually nest routes inside * anyway as that wouldn't make sense, and it seems to ignore IndexRoute inside