Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test to limit allocations of Primer::Classify #166

Merged
merged 20 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
189 changes: 100 additions & 89 deletions lib/primer/classify.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,17 @@ class << self
def call(classes: "", style: nil, **args)
extracted_results = extract_hash(args)

{
class: [validated_class_names(classes), extracted_results[:classes]].compact.join(" ").presence,
style: [extracted_results[:styles], style].compact.join("").presence,
}.merge(extracted_results.except(:classes, :styles))
extracted_results[:class] = [
validated_class_names(classes),
extracted_results.delete(:classes)
].compact.join(" ").presence

extracted_results[:style] = [
extracted_results.delete(:styles),
style
].compact.join("").presence

extracted_results
end

private
Expand Down Expand Up @@ -154,100 +161,104 @@ def validated_class_names(classes)
# Example usage:
# extract_hash({ mt: 4, py: 2 }) => "mt-4 py-2"
def extract_hash(styles_hash)
out = styles_hash.each_with_object({ classes: [], styles: [] }) do |(key, value), memo|
memo = { classes: [], styles: String.new }
manuelpuyol marked this conversation as resolved.
Show resolved Hide resolved
styles_hash.each do |key, value|
next unless VALID_KEYS.include?(key)

if value.is_a?(Array) && !RESPONSIVE_KEYS.include?(key)
raise ArgumentError, "#{key} does not support responsive values"
end

Array(value).each_with_index do |val, index|
next if val.nil?
if value.is_a?(Array)
raise ArgumentError, "#{key} does not support responsive values" if !RESPONSIVE_KEYS.include?(key)

if SPACING_KEYS.include?(key)
if MARGIN_DIRECTION_KEYS.include?(key)
raise ArgumentError, "value of #{key} must be between -6 and 6" if (val < -6 || val > 6)
elsif !((key == :mx || key == :my) && val == :auto)
raise ArgumentError, "value of #{key} must be between 0 and 6" if (val < 0 || val > 6)
end
value.each_with_index do |val, index|
extract_value(memo, key, val, BREAKPOINTS[index])
end
else
extract_value(memo, key, value, BREAKPOINTS[0])
end
end

dasherized_val = val.to_s.dasherize
breakpoint = BREAKPOINTS[index]
memo[:classes] = memo[:classes].join(" ")

if BOOLEAN_MAPPINGS.has_key?(key)
BOOLEAN_MAPPINGS[key][:mappings].map { |m| m[:css_class] if m[:value] == val }.compact.each do |css_class|
memo[:classes] << css_class
end
elsif key == BG_KEY
if val.to_s.starts_with?("#")
memo[:styles] << "background-color: #{val};"
else
memo[:classes] << "bg-#{dasherized_val}"
end
elsif key == COLOR_KEY
if val.to_s.chars.last !~ /\D/
memo[:classes] << "color-#{dasherized_val}"
else
memo[:classes] << "text-#{dasherized_val}"
end
elsif key == DISPLAY_KEY
memo[:classes] << "d#{breakpoint}-#{dasherized_val}"
elsif key == VERTICAL_ALIGN_KEY
memo[:classes] << "v-align-#{dasherized_val}"
elsif key == WORD_BREAK_KEY
memo[:classes] << "wb-#{dasherized_val}"
elsif BORDER_KEYS.include?(key)
memo[:classes] << "border-#{dasherized_val}"
elsif BORDER_MARGIN_KEYS.include?(key)
memo[:classes] << "#{key.to_s.dasherize}-#{val}"
elsif key == BORDER_RADIUS_KEY
memo[:classes] << "rounded-#{val}"
elsif key == DIRECTION_KEY
memo[:classes] << "flex#{breakpoint}-#{dasherized_val}"
elsif key == JUSTIFY_CONTENT_KEY
formatted_value = val.to_s.gsub(/(flex\_|space\_)/, "")
memo[:classes] << "flex#{breakpoint}-justify-#{formatted_value}"
elsif key == ALIGN_ITEMS_KEY
memo[:classes] << "flex#{breakpoint}-items-#{val.to_s.gsub("flex_", "")}"
elsif key == FLEX_KEY
memo[:classes] << "flex-#{val}"
elsif key == FLEX_GROW_KEY
memo[:classes] << "flex-grow-#{val}"
elsif key == FLEX_SHRINK_KEY
memo[:classes] << "flex-shrink-#{val}"
elsif key == ALIGN_SELF_KEY
memo[:classes] << "flex-self-#{val}"
elsif key == WIDTH_KEY || key == HEIGHT_KEY
if val == :fit || val == :fill
memo[:classes] << "#{key}-#{val}"
else
memo[key] = val
end
elsif TEXT_KEYS.include?(key)
memo[:classes] << "text-#{dasherized_val}"
elsif TYPOGRAPHY_KEYS.include?(key)
memo[:classes] << "f#{dasherized_val}"
elsif MARGIN_DIRECTION_KEYS.include?(key) && val < 0
memo[:classes] << "#{key.to_s.dasherize}#{breakpoint}-n#{val.abs}"
elsif key == BOX_SHADOW_KEY
if val == true
memo[:classes] << "box-shadow"
else
memo[:classes] << "box-shadow-#{dasherized_val}"
end
elsif key == VISIBILITY_KEY
memo[:classes] << "v-#{dasherized_val}"
else
memo[:classes] << "#{key.to_s.dasherize}#{breakpoint}-#{dasherized_val}"
end
memo
end

def extract_value(memo, key, val, breakpoint)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept these names the same as from before the refactor, but happy to update them to be more descriptive.

return if val.nil?
if SPACING_KEYS.include?(key)
if MARGIN_DIRECTION_KEYS.include?(key)
raise ArgumentError, "value of #{key} must be between -6 and 6" if (val < -6 || val > 6)
elsif !((key == :mx || key == :my) && val == :auto)
raise ArgumentError, "value of #{key} must be between 0 and 6" if (val < 0 || val > 6)
end
end

{
classes: out[:classes].join(" "),
styles: out[:styles].join(" ")
}.merge(out.except(:classes, :styles))
if BOOLEAN_MAPPINGS.has_key?(key)
BOOLEAN_MAPPINGS[key][:mappings].map { |m| m[:css_class] if m[:value] == val }.compact.each do |css_class|
memo[:classes] << css_class
end
elsif key == BG_KEY
if val.to_s.starts_with?("#")
memo[:styles] << "background-color: #{val};"
else
memo[:classes] << "bg-#{val.to_s.dasherize}"
BlakeWilliams marked this conversation as resolved.
Show resolved Hide resolved
end
elsif key == COLOR_KEY
char_code = val[-1].ord
# Does this string end in a character that is NOT a number?
if char_code >= 48 && char_code <= 57 # 48 is the charcode for 0; 57 is the charcode for 9
memo[:classes] << "color-#{val.to_s.dasherize}"
else
memo[:classes] << "text-#{val.to_s.dasherize}"
end
elsif key == DISPLAY_KEY
memo[:classes] << "d#{breakpoint}-#{val.to_s.dasherize}"
elsif key == VERTICAL_ALIGN_KEY
memo[:classes] << "v-align-#{val.to_s.dasherize}"
elsif key == WORD_BREAK_KEY
memo[:classes] << "wb-#{val.to_s.dasherize}"
elsif BORDER_KEYS.include?(key)
memo[:classes] << "border-#{val.to_s.dasherize}"
elsif BORDER_MARGIN_KEYS.include?(key)
memo[:classes] << "#{key.to_s.dasherize}-#{val}"
elsif key == BORDER_RADIUS_KEY
memo[:classes] << "rounded-#{val}"
elsif key == DIRECTION_KEY
memo[:classes] << "flex#{breakpoint}-#{val.to_s.dasherize}"
elsif key == JUSTIFY_CONTENT_KEY
formatted_value = val.to_s.gsub(/(flex\_|space\_)/, "")
memo[:classes] << "flex#{breakpoint}-justify-#{formatted_value}"
elsif key == ALIGN_ITEMS_KEY
memo[:classes] << "flex#{breakpoint}-items-#{val.to_s.gsub("flex_", "")}"
elsif key == FLEX_KEY
memo[:classes] << "flex-#{val}"
elsif key == FLEX_GROW_KEY
memo[:classes] << "flex-grow-#{val}"
elsif key == FLEX_SHRINK_KEY
memo[:classes] << "flex-shrink-#{val}"
elsif key == ALIGN_SELF_KEY
memo[:classes] << "flex-self-#{val}"
elsif key == WIDTH_KEY || key == HEIGHT_KEY
if val == :fit || val == :fill
memo[:classes] << "#{key}-#{val}"
else
memo[key] = val
end
elsif TEXT_KEYS.include?(key)
memo[:classes] << "text-#{val.to_s.dasherize}"
elsif TYPOGRAPHY_KEYS.include?(key)
memo[:classes] << "f#{val.to_s.dasherize}"
elsif MARGIN_DIRECTION_KEYS.include?(key) && val < 0
memo[:classes] << "#{key.to_s.dasherize}#{breakpoint}-n#{val.abs}"
elsif key == BOX_SHADOW_KEY
if val == true
memo[:classes] << "box-shadow"
else
memo[:classes] << "box-shadow-#{val.to_s.dasherize}"
end
elsif key == VISIBILITY_KEY
memo[:classes] << "v-#{val.to_s.dasherize}"
else
memo[:classes] << "#{key.to_s.dasherize}#{breakpoint}-#{val.to_s.dasherize}"
end
end
end
end
Expand Down
46 changes: 46 additions & 0 deletions test/primer/classify_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,57 @@ def test_does_not_raise_error_when_passing_in_a_primer_css_class_otherwise
assert_generated_class("bg-blue text-center float-left ml-1 ", { classes: "bg-blue text-center float-left ml-1" })
end

def test_limits_allocations
# Warm up allocations
values = {
align_items: :center,
align_self: :center,
bg: :blue,
border: :top,
box_shadow: true,
col: 1,
color: :red,
flex: 1,
float: :left,
font_weight: :bold,
font_size: 1,
height: :fit,
justify_content: :flex_start,
m: 1,
p: 4,
position: :relative,
text_align: :left,
visibility: :hidden,
width: :fit,
underline: true,
vertical_align: true,
}
Primer::Classify.call(**values)

assert_allocations 87, within: 4 do
Primer::Classify.call(**values)
end
end

private

def assert_generated_class(generated_class_name, input)
assert_equal(generated_class_name, Primer::Classify.call(**input)[:class])
end

def refute_generated_class(input)
assert_nil(Primer::Classify.call(**input)[:class])
end

def assert_allocations(count, within:, &block)
GC.disable
total_start = GC.stat[:total_allocated_objects]
yield
total_end = GC.stat[:total_allocated_objects]
GC.enable

total = total_end - total_start

assert_in_delta count, total, within, "Expected between #{count - within} and #{count + within} allocations. Got #{total}"
end
end