Skip to content

processAttr: NaN default value silently breaks identity check #2268

@cursor

Description

@cursor

Problem

processAttr(undefined, defaultValue) returns defaultValue via identity (return defaultValue), but if a feature author passes NaN as the default, downstream comparisons like result === expected will always be false because NaN !== NaN in JavaScript.

This was discovered via a property test that generated NaN as a default value. While feature authors don't currently pass NaN, the function has no guard against it.

Options

  1. Document that NaN is not a supported default — cheapest, matches current reality
  2. Normalize NaN to undefined or null — defensive, prevents subtle bugs if a config accidentally produces NaN (e.g. parseInt('abc'))
  3. No change needed — if the type system (ConfigSetting) already prevents this

Context

Found in PR #2261. The property test was narrowed to realistic defaults as a workaround.

// src/utils.js — processAttr
if (configSetting === undefined) {
    return defaultValue; // if defaultValue is NaN, callers can't compare the result
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    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