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

Removing the catch all route. #397

Closed
T4rk1n opened this issue Sep 20, 2018 · 5 comments
Closed

Removing the catch all route. #397

T4rk1n opened this issue Sep 20, 2018 · 5 comments

Comments

@T4rk1n
Copy link
Contributor

T4rk1n commented Sep 20, 2018

There is a catch all route in dash, it catch any route that doesn't exist and return a 200 with the index. I think this is bad and should be removed as it swallow errors.

dash/dash/dash.py

Lines 214 to 217 in a6388be

# catch-all for front-end routes
add_url(
'{}<path:path>'.format(self.config['routes_pathname_prefix']),
self.index)

Example of unexpected behavior coming from this:
The browser tries to get the /favicon.ico from the server and dash replies OK here's the index. That means the browser will try to get the favicon everytime you load the index since it returns a 200 instead of a 404.

@T4rk1n
Copy link
Contributor Author

T4rk1n commented Sep 20, 2018

cc @plotly/dash

@rmarren1
Copy link
Contributor

Will this break multi-page apps that rely on the Location? (e.g. this)

I always thought this catch-all was to support this functionality, especially in the case where a user navigates directly to a page rather than manipulating the Location via a callback.

@T4rk1n
Copy link
Contributor Author

T4rk1n commented Sep 20, 2018

I see, it's probably because of that, it's a good reason to keep it then.

@chriddyp
Copy link
Member

Yeah, that's why we have it

@ned2
Copy link
Contributor

ned2 commented Sep 23, 2018

I wonder if it might be nice to at least fix the special case of the /favicon.ico route (as discussed in #152). As it can be a little confusing to see the index being returned as the body of the favicon response.

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

4 participants