Docs: bad default for default_protect_from_forgery

[ghiculescu]

On a new Rails 6 app this defaults to true.

It is a bit confusing though, in that here it's true but here it's false. Should I update the code to be consistent also?

[kamipo]

Thank you for the PR, I agree it is not perfectly clear, but it is all documented.

See https://guides.rubyonrails.org/configuring.html#results-of-config-load-defaults.

[ghiculescu]

@kamipo the problem is the docs contradict themselves.

In https://guides.rubyonrails.org/configuring.html#results-of-config-load-defaults it says the default value of default_protect_from_forgery is true.

In https://guides.rubyonrails.org/configuring.html#configuring-action-controller it says the default value of default_protect_from_forgery is false.

As a user this means I have to go and read the Rails code (or boot a new Rails app and check in the console) if I want to be 100% sure what the actual default is.

[ghiculescu]

Here's another example that I'd like to fix:

[kamipo]

Yeah, As I said the above, the load_defaults strategy (legacy settings by default in the codebase, then overwritten by new defaults in railties) is very confusing to me too.

To be honest, I prefer that new defaults by default, then overwritten by legacy defaults if needed, like as Active Record Migration Versioning.

[ghiculescu]

Hmmm okay. I didn't understand how load_defaults works until just now when I read its code. I agree the strategy is confusing. Even more so because we started before Rails 5, when load_defaults was introduced, so we didn't even have it in our codebase until recently.

But even if the strategy is confusing, the docs don't have to be.

If I understand right, what you're saying is:

• The load_defaults section of the docs explains how defaults are overriden when you call load_defaults in your config/application.rb
• All other sections of the docs tell you what the default is in the codebase (and this is the default you'll get if you don't use load_defaults)

Is that correct?

In my opinion that's much too confusing for a casual/new user. It would be much better to assume everyone by now uses load_defaults, and have the defaults consistent throughout the doc. Then in the load_defaults section we can mention that if you don't call that method anywhere in your config/application.rb, you might have different defaults.

[ghiculescu]

btw I can move this over to https://discuss.rubyonrails.org/c/a-may-of-wtfs/8 if it makes more sense to discuss there

[jonathanhefner]

In my opinion that's much too confusing for a casual/new user.

@ghiculescu You are probably right. I think a good approach might be to say something like "This is true by default when the Rails 5.2+ base configuration is loaded."

Yes, feel free to make a thread on the May of WTFs forum! That will let others chime in as to whether this is would be helpful.

[rafaelfranca]

Is that correct?

Correct.

In my opinion that's much too confusing for a casual/new user. It would be much better to assume everyone by now uses load_defaults

With which value? That is what is important and what the documentation can't capture. This is why we have a section to explain what it means when each value is passed to load_defaults.

For a lot of configs the text would look like: This is :x by default when the Rails 5.2 base configuration is loaded, :y when Rails 6.0 is loaded and nil when Rails 5.1- or 6.1 is loaded.

I think that is even more confusing.

An alternative is not to talk at all about which is the default value on the config but only in the Result of config_for` section.

[jonathanhefner]

For a lot of configs the text would look like: This is :x by default when the Rails 5.2 base configuration is loaded, :y when Rails 6.0 is loaded and nil when Rails 5.1- or 6.1 is loaded.

I think listing only the most recent default (at the time of publishing) could be sufficient.

An alternative is not to talk at all about which is the default value on the config but only in the Result of config_for` section.

That could work too. It would be a bit less helpful, but it would still prevent confusion, and wouldn't require any maintenance.

[rafaelfranca]

We could improve the entries to add a link like to see the default value go to this section that points to the right session.

[ghiculescu]

For a lot of configs the text would look like: This is :x by default when the Rails 5.2 base configuration is loaded, :y when Rails 6.0 is loaded and nil when Rails 5.1- or 6.1 is loaded.

I think listing only the most recent default (at the time of publishing) could be sufficient.

An alternative is not to talk at all about which is the default value on the config but only in the Result of config_for` section.

That could work too. It would be a bit less helpful, but it would still prevent confusion, and wouldn't require any maintenance.

I'd be happy with either of these outcomes and am happy to PR either. In both cases it seems like this would remove the contradiction, which is the main thing that bothered me.

[jonathanhefner]

I'd be happy with either of these outcomes and am happy to PR either. In both cases it seems like this would remove the contradiction, which is the main thing that bothered me.

@ghiculescu That would be great! Let's remove the defaults from the various sections, like Rafael suggested. But while you're at it, if you don't mind, could you add another section under "For '5.0'" titled "Baseline defaults", and relocate the defaults there? Then we can leverage the existing structure of the document by changing the "For '5.0':" heading to "For '5.0', baseline defaults from below and:".