Skip to content

Refactor ChordLyricsPair constructor to named arguments (deprecate positional args) #2267

Description

@martijnversluis

Context

ChordLyricsPair has a positional-argument constructor that already carries five parameters:

constructor(
  chords = '',
  lyrics: string | null = null,
  annotation: string | null = null,
  chordObj: Chord | null = null,
  isRhythmSymbol = false,
)

The semantic-token work in the #2207 → #2229 stack adds tokenKind and tokenVariant on top of this, pushing it to seven positional args. Call sites like new ChordLyricsPair(chords, lyrics, annotation, chordObj, isRhythmSymbol, tokenKind, tokenVariant) are hard to read and error-prone (easy to pass an argument in the wrong slot, lots of null/default padding to reach the one you care about).

Proposal

Move the constructor to a single named-options object:

constructor({ chords, lyrics, annotation, chordObj, isRhythmSymbol, tokenKind, tokenVariant }: ChordLyricsPairProperties = {})
  • Keep the positional signature working for now (accept either an options object or the legacy positional args) for backward compatibility.
  • Mark the positional form as @deprecated and emit a deprecate(...) warning, so it can be removed in a future major.
  • #set already takes an options object, so this aligns the constructor with it.

Notes

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions