close
Skip to content

Update scopes guidance in Rails guide#643

Closed
cpjmcquillan wants to merge 1 commit into
mainfrom
update-rails-guide-scopes
Closed

Update scopes guidance in Rails guide#643
cpjmcquillan wants to merge 1 commit into
mainfrom
update-rails-guide-scopes

Conversation

@cpjmcquillan

Copy link
Copy Markdown

Before, we used positive instruction to advocate use of
self.method over ActiveRecord's scope :method DSL.

Using a scope means we can chain methods "for free", and
they always return an ActiveRecord::Relation.

Before, we used positive instruction to advocate use  of 
`self.method` over [ActiveRecord's](1) `scope :method` DSL.

Using a `scope` means we can chain methods "for free", and
they always return an `ActiveRecord::Relation`. 

[1]: https://guides.rubyonrails.org/active_record_querying.html#scopes
@cpjmcquillan

Copy link
Copy Markdown
Author

How do we feel about re-visiting the decision we made initially here?

Are there any pitfalls specific to scopes that I'm missing?

@mike-burns

Copy link
Copy Markdown
Contributor

I've seen some chat of the distinctions between the two but I don't know that I followed. Do you mind summarizing here?

I haven't had any issues with the class methods and would rather keep consistency than change something if we don't have to.

@mike-burns

Copy link
Copy Markdown
Contributor

Not a blocker but, does Solargraph understand Rails scopes? Like, if I jump to definition will it find it?

@jferris

jferris commented Sep 20, 2021

Copy link
Copy Markdown
Contributor

Being able to jump to a definition and otherwise find scopes is one of the reasons I prefer not to use them by default. I follows a general principle to avoid things that are Extra (let, scope, concerns) until the regular patterns available in Ruby are insufficient.

I've found that avoiding these special patterns as your first approach has the upside of almost always finding a root cause deficiency in the design. For example, needing to chain scopes on a class is frequently an indicator that you actually need a query object or you need to split up your table. Large, ad-hoc scope chains are one of the common sources I've seen of slow queries, and they're usually not unit tested in the combinations that will be used in production which makes them a fun source of regressions.

@purinkle

Copy link
Copy Markdown
Contributor

I know that it's outside of the scope of this change, but I would love to read
more details (blog post!) of the following.

I've found that avoiding these special patterns as your first approach has the
upside of almost always finding a root cause deficiency in the design. For
example, needing to chain scopes on a class is frequently an indicator that
you actually need a query object or you need to split up your table. Large,
ad-hoc scope chains are one of the common sources I've seen of slow queries,
and they're usually not unit tested in the combinations that will be used in
production which makes them a fun source of regressions.

@JoelQ

JoelQ commented Sep 23, 2021

Copy link
Copy Markdown
Contributor

@purinkle this article covers the idea of "avoiding these special patterns" https://thoughtbot.com/blog/special-cases

@cpjmcquillan

Copy link
Copy Markdown
Author

Being able to jump to a definition and otherwise find scopes is one of the reasons I prefer not to use them by default. I follows a general principle to avoid things that are Extra (let, scope, concerns) until the regular patterns available in Ruby are insufficient.

I've found that avoiding these special patterns as your first approach has the upside of almost always finding a root cause deficiency in the design. For example, needing to chain scopes on a class is frequently an indicator that you actually need a query object or you need to split up your table. Large, ad-hoc scope chains are one of the common sources I've seen of slow queries, and they're usually not unit tested in the combinations that will be used in production which makes them a fun source of regressions.

Thanks for highlighting some of the general pitfalls of using these sorts of special patterns.

The like the idea that by not being able to chain things by default we force ourselves to think of the potential implications and test cases more explicitly than we otherwise might. I like the neatness and simplicity of scopes, and this hadn't crossed my mind before.

@cpjmcquillan

Copy link
Copy Markdown
Author

Most of the motivation for this PR was to discuss "why not scopes?"

Thanks to everybody who contributed to the discussion - both here and in Slack.

I don't think there is a compelling enough reason to justify updating the guide in this instance now that some concrete drawbacks have been highlighted - so I'm closing for now.

@cpjmcquillan
cpjmcquillan deleted the update-rails-guide-scopes branch September 27, 2021 14:15
@mike-burns

Copy link
Copy Markdown
Contributor

Should we update the docs to link to this discussion from that line?

@cpjmcquillan

Copy link
Copy Markdown
Author

Should we update the docs to link to this discussion from that line?

Good idea! I will open a PR.

stevepolitodesign added a commit to thoughtbot/active-record-recipes that referenced this pull request May 14, 2022
We want this project to be consistent with [our guides](thoughtbot/guides#643).

Issues
------
- Closes #42
stevepolitodesign added a commit to thoughtbot/active-record-recipes that referenced this pull request May 14, 2022
We want this project to be consistent with [our guides](thoughtbot/guides#643).

Issues
------
- Closes #42
@stratigos

Copy link
Copy Markdown
Contributor

Thanks for the discussion here, its enhanced my understanding of this guideline.

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.

6 participants