Skip to content

Commit a6f94c8

Browse files
committed
Improve attribute methods generation
I'm currently working through Ruby warnings in our app, and one of the top source is from ActiveModel Serializer sub classes. Usually they look like: ``` class SomethingSerializer < AMS attributes :title, :body def title # method redefinition warning titleize end end ``` One solution would be to call `attributes` at the end of the method, so that `attributes` skips defining these, but it's quite unatural. Instead we can use the self alias trick to mark these generated methods as OK to redefine. An alternative would be to define them in an included module like Active Model does, but not sure if it's worth it. While I was at it, I improved the way these methods are generated.
1 parent 92ca1d9 commit a6f94c8

File tree

3 files changed

+21
-4
lines changed

3 files changed

+21
-4
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
### [0-9-stable](https://github.com/rails-api/active_model_serializers/compare/v0.9.12...0-9-stable)
44

5+
- Perf
6+
- [#2471](https://github.com/rails-api/active_model_serializers/pull/2471) Generate better attribute accessors, that also don't trigger warnings when redefined. (@byroot)
7+
58
### [v0.9.12 (2024-04-11)](https://github.com/rails-api/active_model_serializers/compare/v0.9.11...v0.9.12)
69

710
- Fix

Gemfile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,10 @@ group :test do
7272
platforms(*(@windows_platforms + [:ruby])) do
7373
if RUBY_VERSION < '2.5' || version < '6'
7474
gem 'sqlite3', '~> 1.3.13'
75+
elsif version >= '8'
76+
gem 'sqlite3', '>= 2'
7577
else
76-
gem 'sqlite3'
78+
gem 'sqlite3', '< 2'
7779
end
7880
end
7981
platforms :jruby do

lib/active_model/serializer.rb

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,15 +90,27 @@ def root_name
9090
end
9191

9292
def attributes(*attrs)
93+
# Use `class_eval` rather than `define_method` for faster access
94+
# and avoid retaining objects in the closure.
95+
# Batch all methods in a single `class_eval` for efficiceny,
96+
# and define all methods on the same line so the eventual backtrace
97+
# properly maps to the `attributes` call.
98+
source = ["# frozen_string_literal: true\n"]
9399
attrs.each do |attr|
94100
striped_attr = strip_attribute attr
95101

96102
@_attributes << striped_attr
97103

98-
define_method striped_attr do
99-
object.read_attribute_for_serialization attr
100-
end unless method_defined?(attr)
104+
unless method_defined?(attr)
105+
source <<
106+
"def #{striped_attr}" <<
107+
"object.read_attribute_for_serialization(#{attr.inspect})" <<
108+
"end" <<
109+
"alias_method :#{striped_attr}, :#{striped_attr}" # suppress method redefinition warning
110+
end
101111
end
112+
caller = caller_locations(1, 1).first
113+
class_eval(source.join(";"), caller.path, caller.lineno - 1)
102114
end
103115

104116
def has_one(*attrs)

0 commit comments

Comments
 (0)