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

Handle 404 better #1596

Open
wants to merge 9 commits into
base: devel
Choose a base branch
from
Open

Handle 404 better #1596

wants to merge 9 commits into from

Conversation

chrisbutler
Copy link
Contributor

@marxo i reverted the devel branch, we can track this here and I can merge if/when there's a working solution

mjmasn and others added 9 commits January 14, 2016 11:05
This is potentially a breaking change for anyone who has not made their client-side routes accessible on the server, because the response will always be 404 for client side routes. The site will still render, but search engines will not index any routes.
This will ensure the 404 status code is replaced with the correct 200 code
Adds a snippet to demonstrate using custom template for 404s
Adds a snippet to demonstrate using custom template for 404s
@marxo
Copy link
Contributor

marxo commented Nov 11, 2017

I tried both a fresh app (Meteor 1.6) and an existing app (1.5.2.2), both working properly with the package on this branch.

firefox_2017-11-11_02-06-28
firefox_2017-11-11_02-07-00

Is there anything specific required to replicate your case?

@chrisbutler
Copy link
Contributor Author

chrisbutler commented Nov 11, 2017

@marxo ah, it's happening for me when the route is declared only on the client

that means there's potentially a significant behavior change here, since we don't explicitly require that all routes be declared in a shared space (https://github.com/iron-meteor/iron-router#routes-on-client-and-server)

@marxo
Copy link
Contributor

marxo commented Nov 11, 2017

@chrisbutler Yep, now I see it. Should controller.willBeHandledOnClient be true for client-side only routes? Specifically this one: https://github.com/iron-meteor/iron-router/blob/devel/lib/router_server.js#L78

@marxo
Copy link
Contributor

marxo commented Nov 11, 2017

@chrisbutler I'm looking at the middleware stack and as far as I can see when I move the route file to client I always get:

_handlersForEnv: { client: false, server: false }

in the dispatch call. However, when I move it to the shared space, I get

_handlersForEnv: { client: true, server: false }

for client routes.

Could this be a bug with the middleware stack? If I move the route definition to the server the route fails.

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