Skip to content

Commit 2a52432

Browse files
committed
use an array to store presenter dependencies and keep them sorted
sorted_set stopped working in Ruby after internal Set changes (changes description: https://bugs.ruby-lang.org/issues/21216, https://rubyreferences.github.io/rubychanges/4.0.html#set). Set subclasses cannot use `@hash` anymore, which breaks the way `sorted_set` works (it replaces `@hash` with a red-black tree from `rbtree`). We need to preserve an ordered list of dependencies to build the same cache key every time (introduced in this commit: e132533). Since these lists are pretty short, and updates of these lists don’t happen on a hot path, an array that preserves the order is good enough. We will maintain uniqueness and sort order in both methods that update this array.
1 parent 4f9ecd0 commit 2a52432

9 files changed

Lines changed: 45 additions & 45 deletions

CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
### Unreleased
22
* Add tests with Rails 7.2 and 8.0
3-
* Add tests with Ruby 3.4
3+
* Add tests with Ruby 3.4 and 4.0
44
* Drop support for Ruby < 3.2
5+
* Curly::Presenter#dependencies now return an Array instead of a SortedSet. Entries in this array will still be sorted.
6+
* Drops dependency on sorted_set
57

68
### Curly 3.4.0 (July 2, 2024)
79
* Drop upper limit on Rails, test with Rails main.

curly-templates.gemspec

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ Gem::Specification.new do |s|
1919
s.required_ruby_version = ">= 3.2"
2020

2121
s.add_dependency("actionpack", ">= 6.1")
22-
s.add_dependency("sorted_set")
2322

2423
s.add_development_dependency("rake")
2524
s.add_development_dependency("rspec", ">= 3")

gemfiles/rails6.1.gemfile.lock

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ PATH
1212
specs:
1313
curly-templates (3.4.0)
1414
actionpack (>= 6.1)
15-
sorted_set
1615

1716
GEM
1817
remote: https://rubygems.org/
@@ -147,7 +146,6 @@ GEM
147146
rake (>= 12.2)
148147
thor (~> 1.0)
149148
rake (13.1.0)
150-
rbtree (0.4.6)
151149
redcarpet (3.6.0)
152150
rspec (3.12.0)
153151
rspec-core (~> 3.12.0)
@@ -170,10 +168,6 @@ GEM
170168
rspec-mocks (~> 3.12)
171169
rspec-support (~> 3.12)
172170
rspec-support (3.12.1)
173-
set (1.0.3)
174-
sorted_set (1.0.3)
175-
rbtree
176-
set (~> 1.0)
177171
sprockets (4.2.1)
178172
concurrent-ruby (~> 1.0)
179173
rack (>= 2.2.4, < 4)

gemfiles/rails7.0.gemfile.lock

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ PATH
1212
specs:
1313
curly-templates (3.4.0)
1414
actionpack (>= 6.1)
15-
sorted_set
1615

1716
GEM
1817
remote: https://rubygems.org/
@@ -153,7 +152,6 @@ GEM
153152
thor (~> 1.0)
154153
zeitwerk (~> 2.5)
155154
rake (13.1.0)
156-
rbtree (0.4.6)
157155
redcarpet (3.6.0)
158156
rspec (3.12.0)
159157
rspec-core (~> 3.12.0)
@@ -176,10 +174,6 @@ GEM
176174
rspec-mocks (~> 3.12)
177175
rspec-support (~> 3.12)
178176
rspec-support (3.12.1)
179-
set (1.0.3)
180-
sorted_set (1.0.3)
181-
rbtree
182-
set (~> 1.0)
183177
stackprof (0.2.25)
184178
thor (1.3.0)
185179
timeout (0.4.1)

gemfiles/rails7.1.gemfile.lock

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ PATH
1212
specs:
1313
curly-templates (3.4.0)
1414
actionpack (>= 6.1)
15-
sorted_set
1615

1716
GEM
1817
remote: https://rubygems.org/
@@ -178,7 +177,6 @@ GEM
178177
thor (~> 1.0, >= 1.2.2)
179178
zeitwerk (~> 2.6)
180179
rake (13.1.0)
181-
rbtree (0.4.6)
182180
rdoc (6.6.0)
183181
psych (>= 4.0.0)
184182
redcarpet (3.6.0)
@@ -205,10 +203,6 @@ GEM
205203
rspec-mocks (~> 3.12)
206204
rspec-support (~> 3.12)
207205
rspec-support (3.12.1)
208-
set (1.0.3)
209-
sorted_set (1.0.3)
210-
rbtree
211-
set (~> 1.0)
212206
stackprof (0.2.25)
213207
stringio (3.0.9)
214208
thor (1.3.0)

gemfiles/rails7.2.gemfile.lock

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ PATH
1212
specs:
1313
curly-templates (3.4.0)
1414
actionpack (>= 6.1)
15-
sorted_set
1615

1716
GEM
1817
remote: https://rubygems.org/
@@ -176,7 +175,6 @@ GEM
176175
thor (~> 1.0, >= 1.2.2)
177176
zeitwerk (~> 2.6)
178177
rake (13.2.1)
179-
rbtree (0.4.6)
180178
rdoc (6.10.0)
181179
psych (>= 4.0.0)
182180
redcarpet (3.6.0)
@@ -204,10 +202,6 @@ GEM
204202
rspec-support (~> 3.13)
205203
rspec-support (3.13.2)
206204
securerandom (0.4.1)
207-
set (1.1.1)
208-
sorted_set (1.0.3)
209-
rbtree
210-
set (~> 1.0)
211205
stackprof (0.2.26)
212206
stringio (3.1.2)
213207
thor (1.3.2)
@@ -243,4 +237,4 @@ DEPENDENCIES
243237
yard-tomdoc
244238

245239
BUNDLED WITH
246-
2.5.17
240+
4.0.3

gemfiles/rails8.0.gemfile.lock

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ PATH
1212
specs:
1313
curly-templates (3.4.0)
1414
actionpack (>= 6.1)
15-
sorted_set
1615

1716
GEM
1817
remote: https://rubygems.org/
@@ -176,7 +175,6 @@ GEM
176175
thor (~> 1.0, >= 1.2.2)
177176
zeitwerk (~> 2.6)
178177
rake (13.2.1)
179-
rbtree (0.4.6)
180178
rdoc (6.10.0)
181179
psych (>= 4.0.0)
182180
redcarpet (3.6.0)
@@ -204,10 +202,6 @@ GEM
204202
rspec-support (~> 3.13)
205203
rspec-support (3.13.2)
206204
securerandom (0.4.1)
207-
set (1.1.1)
208-
sorted_set (1.0.3)
209-
rbtree
210-
set (~> 1.0)
211205
stackprof (0.2.26)
212206
stringio (3.1.2)
213207
thor (1.3.2)
@@ -244,4 +238,4 @@ DEPENDENCIES
244238
yard-tomdoc
245239

246240
BUNDLED WITH
247-
2.5.17
241+
4.0.3

lib/curly/presenter.rb

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
require 'curly/presenter_name_error'
2-
require "sorted_set"
32

43
module Curly
54

@@ -246,23 +245,23 @@ def available_components
246245
# Posts::ShowPresenter.dependencies
247246
# #=> ['posts/comment', 'posts/comment_form']
248247
#
249-
# Returns a Set of String view paths.
248+
# Returns an Array of sorted view paths.
250249
def dependencies
251250
# The base presenter doesn't have any dependencies.
252-
return SortedSet.new if self == Curly::Presenter
251+
return [] if self == Curly::Presenter
253252

254-
@dependencies ||= SortedSet.new
255-
@dependencies.union(superclass.dependencies)
253+
@dependencies ||= []
254+
@dependencies = @dependencies.union(superclass.dependencies).uniq.sort
256255
end
257256

258-
# Indicate that the presenter depends a list of other views.
257+
# Indicate that the presenter depends on a list of other views.
259258
#
260-
# deps - A list of String view paths that the presenter depends on.
259+
# dependencies - A list of String view paths that the presenter depends on.
261260
#
262-
# Returns nothing.
261+
# Returns updated Array of String view paths.
263262
def depends_on(*dependencies)
264-
@dependencies ||= SortedSet.new
265-
@dependencies.merge(dependencies)
263+
@dependencies ||= []
264+
@dependencies = @dependencies.union(dependencies).uniq.sort
266265
end
267266

268267
# Get or set the version of the presenter.

spec/presenter_spec.rb

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,29 @@ class SubNestedPresenter < Curly::Presenter
226226
expect(cache_key).to eq "Foo::BarPresenter/42/Foo::BumPresenter/1337"
227227
end
228228

229+
it "includes the cache keys of all presenters in the dependency list" do
230+
presenter = Class.new(Curly::Presenter) do
231+
version 42
232+
depends_on 'foo/bum'
233+
depends_on 'foo/aum'
234+
end
235+
236+
dependency = Class.new(Curly::Presenter) do
237+
version 1337
238+
end
239+
240+
dependency_2 = Class.new(Curly::Presenter) do
241+
version 1338
242+
end
243+
244+
stub_const("Foo::BarPresenter", presenter)
245+
stub_const("Foo::BumPresenter", dependency)
246+
stub_const("Foo::AumPresenter", dependency_2)
247+
248+
cache_key = Foo::BarPresenter.cache_key
249+
expect(cache_key).to eq "Foo::BarPresenter/42/Foo::AumPresenter/1338/Foo::BumPresenter/1337"
250+
end
251+
229252
it "uses the view path of a dependency if there is no presenter for it" do
230253
presenter = Class.new(Curly::Presenter) do
231254
version 42
@@ -249,7 +272,14 @@ class SubNestedPresenter < Curly::Presenter
249272
Curly::Presenter.dependencies
250273
parent = Class.new(Curly::Presenter) { depends_on 'foo' }
251274
presenter = Class.new(parent) { depends_on 'bar' }
252-
expect(presenter.dependencies.to_a).to match_array ['foo', 'bar']
275+
expect(presenter.dependencies).to eq ['bar', 'foo']
276+
end
277+
278+
it "doesn’t include duplicates" do
279+
Curly::Presenter.dependencies
280+
parent = Class.new(Curly::Presenter) { depends_on 'foo' }
281+
presenter = Class.new(parent) { depends_on 'bar'; depends_on 'foo' }
282+
expect(presenter.dependencies).to eq ['bar', 'foo']
253283
end
254284
end
255285
end

0 commit comments

Comments
 (0)