Skip to content

Conversation

@denisdefreyne
Copy link
Member

@denisdefreyne denisdefreyne commented Oct 3, 2022

Detailed description

This adds support for queries like @items.where(kind: 'article'), which would return a collection of all items where the kind attribute is "article".

In behavior, this is nearly identical to @items.select { _1[:kind] == 'article' }, but is more friendly to the dependency tracker and outdatedness checker. For example:

  • This creates only a single dependency, onto the item collection, rather than individual dependencies for each item. This cuts down on the number of dependencies. A lower number of dependencies means the dependency is smaller, and can be loaded/stored more quickly. For large sites, with many thousands of items, this can make a significant difference.

  • This prevents items from being marked outdated if their kind attribute changes from something that isn’t "article" to something else that isn’t "article" either (e.g. from "note" to "thought"). This can reduce the number of redundant recompiles.

For now, this feature is available only behind a feature flag. Set the NANOC_FEATURES environment variable to where to enable usage of .where(…). (Alternatively, set it to all to enable all feature flags.)

To do

  • Tests
  • Refactoring
  • Documentation (pending -- will be added later)
  • Feature flags

Related issues

n/a

@denisdefreyne denisdefreyne force-pushed the denis/identifiable-collection-where branch 2 times, most recently from dde6275 to 3566e3a Compare October 3, 2022 09:16
@opatry
Copy link
Contributor

opatry commented Oct 3, 2022

Sounds great!! I use this kind of stuff all the time 😅

Very much appreciated improvement proposal 👍

@denisdefreyne denisdefreyne force-pushed the denis/identifiable-collection-where branch 3 times, most recently from 2830b6a to 637a1fa Compare October 3, 2022 14:35
@denisdefreyne denisdefreyne force-pushed the denis/identifiable-collection-where branch 2 times, most recently from 352ce99 to f40aa20 Compare October 8, 2022 12:25
@denisdefreyne denisdefreyne force-pushed the denis/identifiable-collection-where branch from f40aa20 to d4b3978 Compare October 8, 2022 12:31
@denisdefreyne denisdefreyne marked this pull request as ready for review October 8, 2022 12:32
@denisdefreyne denisdefreyne force-pushed the denis/identifiable-collection-where branch from d4b3978 to ea47861 Compare October 8, 2022 12:37
@denisdefreyne denisdefreyne merged commit 3e0b3a8 into main Oct 8, 2022
@denisdefreyne denisdefreyne deleted the denis/identifiable-collection-where branch October 8, 2022 12:45
@opatry
Copy link
Contributor

opatry commented Mar 4, 2025

Hi @denisdefreyne 👋

Do you plan to update this feature to stable anytime soon, not requiring the use of NANOC_FEATURES?

Thanks.

@denisdefreyne
Copy link
Member Author

Hey @opatry, thanks for the poke!

This never got released because I ultimately wasn’t happy with how the #where method ended up looking. What I had in mind was something more powerful, like a generic “query” method, but I never ended up working on that either.

Let me give this a bit more thought and I’ll follow up.

@opatry
Copy link
Contributor

opatry commented Mar 5, 2025

Thanks for the update.

I understand your ultimate goal.

For the record, if it helps adding use cases, on my side I use:

@items.select { |item| item.identifier =~ '/someprefix/**/*' }
@items.select { |item| item.identifier =~ '/someotherprefix/*' }
@items.select { |item| item.identifier =~ '/someotherprefix/blah-*' }
@items.select { |item| item[:extension] == 'md' }
@items.select { |item| %i[post page].include?(item[:kind]) }
@items.select { |item| item[:kind] == :page }

Don't know if a #where contract could suit all these needs, just sharing if it helps.

@denisdefreyne
Copy link
Member Author

For the first few use cases, there is #find_all too:

@items.find_all('/someprefix/**/*')
@items.find_all('/someotherprefix/*')
@items.find_all('/someotherprefix/blah-*')
@items.find_all('/**/*.md')

#find_all is cached and might thus yield a performance boost, too.

@denisdefreyne
Copy link
Member Author

This illustrates part of the problem, in fact: #where and #find_all are not combinable.

For example, it would be great to be able to do @items.find_all('/notes/*').where(draft: true) — but that’s not possible with #where at the moment.

@denisdefreyne
Copy link
Member Author

Having given this a bit more thought, I think it’d need to be able to chain queries, like @items.find_all('/notes/*').where(draft: true). Or perhaps need a generic #query method.

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