Skip to content

Tighten rotation prop handling to reject or normalize permissive values #561

Description

@jasonlundien

Background

Discovered while reviewing PR #560 (the fix for #336 to accept 0 as rotation). Copilot flagged it in two related comments:

The rotation prop validator uses Number.parseInt, which is overly permissive: values like '00', '090', '90deg', or 90.5 will currently validate because they parse to an allowed integer. With 0 now allowed, '00' / '0deg' also become valid and then don’t match the classList’s special-case checks. Consider validating exact allowed values instead (e.g., allow only the exact numbers 0/90/180/270 or the exact strings '0'/'90'/'180'/'270', or use Number(value) plus a strict string round-trip check) so invalid representations don’t pass validation.

The rotation prop validator uses Number.parseInt, which is overly permissive: values like '00', '090', '90deg', or 90.5 will currently validate because they parse to an allowed integer. With 0 now allowed, '00' / '0deg' also become valid and then don’t match the classList’s special-case checks. Consider validating exact allowed values instead (e.g., allow only the exact numbers 0/90/180/270 or the exact strings '0'/'90'/'180'/'270', or use Number(value) plus a strict string round-trip check) so invalid representations don’t pass validation.

This is pre-existing behavior unrelated to #336 — it has shipped in every 3.x release. Filing it here so #560 can stay focused on the 0 fix.

Problem

The rotation prop validator at src/components/FontAwesomeIcon.js:80 is:

validator: (value) => [0, 90, 180, 270].indexOf(Number.parseInt(value, 10)) > -1

Number.parseInt is forgiving — it strips trailing non-numeric characters, ignores leading zeros, and truncates floats. As a result, all of these pass validation and propagate to the class generator in src/utils.js:18:

Input Number.parseInt(input, 10) Validator Resulting class
'00' 0 ✓ accepted fa-rotate-00 (does not exist)
'090' 90 ✓ accepted fa-rotate-090 (does not exist)
'90deg' 90 ✓ accepted fa-rotate-90deg (does not exist)
90.5 90 ✓ accepted fa-rotate-90.5 (does not exist)
'0deg' 0 ✓ accepted fa-rotate-0deg (does not exist)

In every case the user gets no rotation, no warning, and a junk DOM class — the worst possible failure mode because they think it's working.

Two possible fixes

Option A — Tighten the validator (strict, breaking)

- validator: (value) => [0, 90, 180, 270].indexOf(Number.parseInt(value, 10)) > -1
+ validator: (value) => [0, 90, 180, 270, '0', '90', '180', '270'].indexOf(value) > -1

indexOf uses ===, so non-canonical values are rejected with a clear Vue prop validator warning at the call site.

Pro: Surfaces the bug at development time. User knows immediately what they did wrong.
Con: Breaking. Anyone currently passing '90deg', '090', or 90.5 (intentionally or not) will start seeing warnings.

Option B — Normalize in classList() (non-breaking)

Keep the validator permissive but normalize the value before constructing the class name:

const normalizedRotation = props.rotation === null
  ? null
  : Number.parseInt(props.rotation, 10)

// in the classes object:
[\`fa-rotate-\${normalizedRotation}\`]: normalizedRotation !== null && normalizedRotation !== 0,

Pro: Non-breaking. '90deg' still produces fa-rotate-90 (the class FontAwesome's CSS actually has).
Con: Silently accepts malformed input — the user never finds out they had a typo.

Recommendation

Option A is the cleaner long-term fix and aligns with how other props in this component validate (e.g. flip, pull, size use exact-string whitelists). The breaking change can be communicated in a minor-version changelog, especially since the current permissive behavior produces broken output anyway — anyone affected was already seeing a silent bug.

Option B is the safer middle-ground if backward compatibility is a hard requirement.

Acceptance criteria

  • Decide between Option A and Option B (or a hybrid — e.g. normalize in utils.js for compatibility plus a one-time deprecation warning for non-canonical input)
  • Tests covering rejection (or normalization) of '00', '090', '90deg', 90.5
  • Note in CHANGELOG.md, especially if Option A is chosen

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions