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
Make Default Index Page Dynamic #7771
Conversation
Dynamic error pages have many benefits over static, for instance they can use the default layout if desired. By generating dynamic error pages by default we can encourage developers to take advantage of this already existing feature and build better sites. In addition to moving people towards dynamically generating their error pages we can add prompts to the default pages. One example is telling devs how to get to their logs. I have added erb to `errors/500.html.erb` that I believe will be extremely helpful to new developers. While walking users through a deployment (such as with http://railsgirls.com/materials) they often are not familiar with how to retrieve logs on a given system. I've added a dynamic section that will look for an environment variable `CHECK_LOG_INSTRUCTIONS` and if present show a corresponding message. I can then work with @hone to get a debug log message into the [ruby buildpack](https://github.com/heroku/heroku-buildpack-ruby) something like `$ heroku logs --tail`. While we've added a number of great error messages to Rails, helping developers find those error messages is crucial to their sustained success. This isn't just for new devs either. Experienced Rails developers who are new to a system might not know how to access its logs. By relying on an ENV var we're not coupling the message to any one provider, and if no message is present we fall back on the default error page. It would be the responsibility of providers to set this message on a given system. Moving in this direction we should also port over `public/index.html` to a dynamic page. This change can be found in a separate PR with it's own set of benefits rails#7771 Screenshot when running production with `CHECK_LOG_INSTRUCTIONS` set: 
| * Remove highly uncommon `config.assets.manifest` option for moving the manifest path. | ||
| This option is now unsupported in sprockets-rails. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably shouldn't mess with other CHANGELOG lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change is fine. I hate these whitespaces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whoops, sorry ![]()
|
This is a big change, but I am |
|
|
|
It does teach the way public/ works early on. Although perhaps simply following the advice "delete your index.html" doesn't actually teach anything. |
| # root to: 'welcome#index' | ||
| root to: 'pages#index' | ||
|
|
||
| resources :pages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add :only => :show? Since there are no other actions on that controller?
|
My concern is the number of steps it takes to get to a blank project. Now instead of just removing one file, I need to remove a controller, view directory, and the routes? What if we generate no additional files, and simply have one line in the routes file: root to: welcome_aboardThis could be a rack handler which responds with the dynamic content internally (like the existing |
|
I agree with @ryanb on this one. This is adding a lot of noise for people that actually want to build a new application and get stuff of the way in order to make it easier for people just starting. Both audiences matter but in this case the amount of noise added is not worth it. |
|
@ryanb abstracing this away from the user even further would be the oposite intention of this PR. Why not use Rails features to build rails? @josevalim we can modify the --without-index flag to not produce these files. Not having a default root is a sore spot in the |
|
@schneems I'm okay with it if it is intended to be a standardized way to have a home page. In that case I suggest renaming it to WelcomeController and removing the show action so there's no need for the I like the idea of a root route by default. This would allow gems to use |
|
@ryanb I chose 1.9.3dev :001 > "welcome".pluralize
=> "welcomes"
1.9.3dev :002 > "home".pluralize
=> "homes"
1.9.3dev :003 > "page".pluralize
=> "pages" At the end of the day i'm impartial to the naming as long as we get rid of |
|
@schneems Does the controller need to be plural? If it is just displaying the home page I don't see why. Pages does make sense if you are using it to handle other pages (about/privacy as you mentioned). In that case how about using this route instead of get 'pages/:page', to: 'pages#show', as: 'page'If this is intended to be an educational example I don't think I still think this is too complex overall for the default app generator. |
|
@ryanb @schneems Why not just have the file be called welcome.html and redirect root there? The most confusing part of index.html is its name because it then overrides every index until you figure out how to delete it. Post-asset pipeline, the role of public/ is greatly diminished so don't send new users through a loop to delete unnecessary files. |
|
+1 for this I've probably made 100 new rails apps and still forget to do this also, I think rails should set a convention for what a 'home' or 'root' controller/action should be |
|
Convention is: models are singular, controllers are plural, and therefore view folders are pluralized. Generating files that goes against that convention seems bad for a convention-over-configuration advocacy standpoint. A controller-view-route is the minimum required to deliver a generated webpage (with a template) using Rails. We can remove the |
|
Updated routes, removed show action and associated routes from |
| class PagesController < ApplicationController | ||
|
|
||
| def index | ||
| render :layout => false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, use 1.9 hash syntax here. Thanks.
|
@schneems , are you still working on this? |
Rails is a dynamic framework that serves a static `index.html` by default. One of my first questions ever on IRC was solved by "delete your index.html". This file is a source of confusion when starting as it over-rides any set "root" in the routes yet it itself is not listed in the routes. By making the page dynamic by default we can eliminate this confusion and show people how to set the root on their project all at the same time. I encounter this question any time I am helping someone new learn Rails, and it is advice repeated again and again from our comments in rails/rails to external tutorials. We can remove this extra step and show people how to use rails root in the routes by allowing the index to be dynamic. This makes it easier to understand how to replace the root if desired, or to modify the index once generated since all rails helper methods are available. Eventually we can refactor this page to use standard Rails helpers instead of pure static html. Screenshot: 
|
I updated the hash to 1.9 syntax, rebased out the trailing whitespace removal. I would like to open up the discussion on this PR again. I still believe we should use Rails features to deliver this home page out of the box. A question to those who might oppose this change: If we already had a rails controller and view rendering the default index page what benefits would we gain by moving to a static home page instead? Does the static page provide functionality above and beyond a dynamically rendered home/welcome page? |
|
|
|
Just wanted to comment that we already had this discussion but I never coded it. @schneems check 5c1109a#commitcomment-1307473 and please do it :) |
This is an alternative implementation to rails#7771 thanks to the advice of @spastorino Rails is a dynamic framework that serves a static index.html by default. One of my first questions ever on IRC was solved by simply deleting my public/index.html file. This file is a source of confusion when starting as it over-rides any set "root" in the routes yet it itself is not listed in the routes. By making the page dynamic by default we can eliminate this confusion. This PR moves the static index page to an internal controller/route/view similar to `rails/info`. When someone starts a rails server, if no root is defined, this route will take over and the "dynamic" index page from rails/welcome_controller will be rendered. These routes are only added in development. If a developer defines a root in their routes, it automatically takes precedence over this route and will be rendered, with no deleting of files required. In addition to removing this source of confusion for new devs, we can now use Rails view helpers to build and render this page. While not the primary intent, the added value of "dogfooding" should not be under-estimated. The prior PR rails#7771 had push-back since it introduced developer facing files. This PR solves all of the same problems, but does not have any new developer facing files (it actually removes one). cc/ @wsouto, @dickeyxxx, @tyre, @ryanb, @josevalim, @maxim, @subdigital, @steveklabnik ATP Railties and Actionpack.
|
closing in favor of #8468 |
This is an alternative implementation to rails#7771 thanks to the advice of @spastorino Rails is a dynamic framework that serves a static index.html by default. One of my first questions ever on IRC was solved by simply deleting my public/index.html file. This file is a source of confusion when starting as it over-rides any set "root" in the routes yet it itself is not listed in the routes. By making the page dynamic by default we can eliminate this confusion. This PR moves the static index page to an internal controller/route/view similar to `rails/info`. When someone starts a rails server, if no root is defined, this route will take over and the "dynamic" index page from rails/welcome_controller will be rendered. These routes are only added in development. If a developer defines a root in their routes, it automatically takes precedence over this route and will be rendered, with no deleting of files required. In addition to removing this source of confusion for new devs, we can now use Rails view helpers to build and render this page. While not the primary intent, the added value of "dogfooding" should not be under-estimated. The prior PR rails#7771 had push-back since it introduced developer facing files. This PR solves all of the same problems, but does not have any new developer facing files (it actually removes one). cc/ @wsouto, @dickeyxxx, @tyre, @ryanb, @josevalim, @maxim, @subdigital, @steveklabnik ATP Railties and Actionpack.
Rails is a dynamic framework that serves a static
index.htmlby default. One of my first questions ever on IRC was solved by "delete your index.html". This file is a source of confusion when starting as it over-rides any set "root" in the routes yet it itself is not listed in the routes. By making the page dynamic by default we can eliminate this confusion and show people how to set the root on their project all at the same time.I encounter this question any time I am helping someone new learn Rails, and it is advice repeated again and again from our comments in rails/rails to external tutorials. We can remove this extra step and show people how to use rails root in the routes by allowing the index to be dynamic.
In addition to helping new programmers, this will help Rails engine gem's that need to redirect to a url. Many of them would like to use
root_urlsuch as devise, but cannot count on it being there. This change adds in a default page for the root to avoid this issue and to standardize on a place for custom landing pages for applications. If no custom landing page is desired one can easily remove the route, controller, and view. Alternatively they could change the root in routes and leave the other files if they might want a custom landing page eventually.Screenshot: