Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@

## [Unreleased]

### Fixed

- **Declaration harvest unwraps trailing `.freeze`.** `ICONS = { "x" => :car }.freeze`
(and `%i[a b].freeze`) used to yield a Prism `CallNode`, so hash/array values
were never collected. Cross-file dynamics (`PhosphorIcon(@icon)` in a shared
component + names only in a frozen `ICONS` map) then lost those icons at
`glyphs:prune_icons`. `collect_declaration` now unwraps zero-arg `.freeze`
(and parentheses) before walking the value.

### Added

- **Dynamic icon calls are now resolved from source.** `SourceScanner` no longer
Expand All @@ -13,7 +22,8 @@
- _declaration-based_: literals in icon-declaration positions anywhere (a hash
pair keyed `/icon/i`, or a constant named `/ICON/`) are kept for every
dynamically-rendered library, closing the cross-file gap (e.g. a notifier
`ICON = :bell` constant rendered from a view).
`ICON = :bell` constant rendered from a view). Frozen maps
(`ICONS = { … }.freeze`) are included — see Fixed above.

This makes `keep_icons` a last-resort escape hatch (DB/ENV/gem-chrome names)
rather than the primary mechanism. Only literals are harvested, so the scanner
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ The scanner resolves them from source, so you rarely need `keep_icons` at all:
ternaries (`@open ? "caret-up" : "caret-down"`), `case/when`, and locals.
- **Declaration-based** — literals in icon-declaration positions *anywhere* — a
hash pair keyed like an icon (`icon: :gear`, `menu_icon: "house"`) or a
constant named like one (`ICON = :bell`, `STATUS_ICONS = { .. => :warning }`) —
are kept for every dynamically-rendered library, so a name declared in one
file and rendered from another survives.
constant named like one (`ICON = :bell`, `STATUS_ICONS = { .. => :warning }`,
including trailing `.freeze`) — are kept for every dynamically-rendered
library, so a name declared in one file and rendered from another survives.

Only literals are harvested, so the scanner never invents a reference; the
worst case is keeping a coincidentally icon-named string, which the post-prune
Expand Down
35 changes: 35 additions & 0 deletions lib/glyphs/source_scanner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,11 @@ def collect_literal(set, node)

# Harvests icon names from a declaration value: a bare literal, an array of
# literals (`ICON = %i[a b]`), or a hash's values (`{ "sms" => :device }`).
# Trailing `.freeze` is unwrapped so `ICONS = { … }.freeze` still harvests —
# without this, cross-file dynamics (`PhosphorIcon(@icon)` + names only in a
# frozen `ICONS` hash) lose those names at prune.
def collect_declaration(value_node)
value_node = unwrap_declaration_wrappers(value_node)
case value_node
when Prism::SymbolNode, Prism::StringNode
collect_literal(@declaration_literals, value_node)
Expand All @@ -356,6 +360,37 @@ def collect_declaration(value_node)
value_node.elements.grep(Prism::AssocNode).each { |assoc| collect_declaration(assoc.value) }
end
end

# Peel zero-arg `.freeze` and parentheses so the underlying Hash/Array/literal
# is what declaration harvest walks.
def unwrap_declaration_wrappers(value_node)
loop do
case value_node
when Prism::CallNode
break unless freeze_call?(value_node)

value_node = value_node.receiver
when Prism::ParenthesesNode
value_node = value_node.body
when Prism::StatementsNode
# `(expr)` → StatementsNode with one child; multi-statement bodies are
# not valid constant values, so take the sole statement when present.
break unless value_node.body.size == 1

value_node = value_node.body.first
else
break
end
end
value_node
end

# `receiver.freeze` with no args — the common immutable-constant pattern.
def freeze_call?(node)
node.name == :freeze &&
node.receiver &&
(node.arguments.nil? || node.arguments.arguments.empty?)
end
end
end
end
16 changes: 16 additions & 0 deletions spec/fixtures/source/app/components/frozen_icons_map.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

# Cross-file declaration harvest: icon names live only here, rendered
# dynamically from `selectable_row.rb` via `PhosphorIcon(@icon)`.
# The trailing `.freeze` used to hide HashNode values from declaration
# harvest (Prism sees a CallNode). Not loaded at runtime — only parsed
# by SourceScanner specs.
class FrozenIconsMap
ICONS = {
"driving_license" => :car,
"residence_permit" => :identification_badge,
}.freeze

# Array form with freeze should harvest too.
STATUS_ICONS = %i[warning check_circle].freeze
end
16 changes: 16 additions & 0 deletions spec/fixtures/source/app/components/selectable_row.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

# Shared row that renders an icon passed in from another file. Forces a
# dynamic phosphor keep-set so FrozenIconsMap declarations are exercised
# as global declaration harvests. Not loaded at runtime — only parsed.
class SelectableRow < Phlex::HTML
include Glyphs

def initialize(icon:)
@icon = icon
end

def view_template
PhosphorIcon(@icon)
end
end
13 changes: 13 additions & 0 deletions spec/glyphs/source_scanner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,19 @@ def ref(library, variant, name)
expect(keeps[:phosphor]).to include("bell-ringing")
end

# `ICONS = { "driving_license" => :car, … }.freeze` in frozen_icons_map.rb,
# rendered via PhosphorIcon(@icon) in selectable_row.rb. Without unwrapping
# the trailing `.freeze` CallNode, declaration harvest sees nothing and
# cross-file dynamics prune those SVGs (production Icons::IconNotFound).
it "keeps ICON* hash and array values even when the constant is .freeze'd" do
expect(keeps[:phosphor]).to include(
"car",
"identification-badge",
"warning",
"check-circle"
)
end

# A library with no dynamic call gets no dynamic keeps — heroicons here is
# only ever called with literal names, so declaration literals don't leak
# into it.
Expand Down
Loading