Skip to content

Should the impl objects of non-ISO calendars be ES6 classes too? #119

@justingrant

Description

@justingrant

Context: built-in calendar implementations are contained in plain objects, not ES6 class instances. The ISO calendar uses a singleton object, while non-ISO calendars have a singleton object that's cloned for each of the 18 supported non-ISO calendars and amended with a single helper property that differs for each calendar and encapsulates per-calendar data and logic.

The reason for splitting things in this way was originally to keep the ISO and non-ISO implementations as similar as possible, and to make it really clear when reading code that when you see helper. it means that code is doing something where behavior may vary between non-ISO calendars.

#113 converted all the helper objects into ES6 class instances. They're still singletons at runtime, but making them classes made the code easier to understand and (hopefully!) maintain.

@12wrigja made a suggestion in #113 (comment): should the top-level implementation objects also be ES6 classes?

There are three options:

  1. Leave as-is
  2. Convert both the ISO and non-ISO XxxImpl instances to classes (either anonymous class expressions or named classes which are only instantiated once) but otherwise leave the impl vs. helper split as-is.
  3. Convert ISO to a class like (2), but for non-ISO we'd both convert to a class and also remove the "impl" vs. "helper" split. If we did this, then we'd merge the nonIsoImpl object with the HelperBase class to get a single abstract base class for all non-ISO calendars, which would then be specialized with derived classes for each of the 18 non-ISO calendars, per the inheritance structure described in Refactor non-ISO calendars to use ES6 classes #113.

I don't have a strong opinion between these options.

An interesting related question is whether #113 and/or the changes proposed above should be ported back to proposal-temporal. Regardless of the answer, (2) or (3) above would introduce more deviation from proposal-temporal and therefore probably more pressure to port changes over. But, honestly, the changes in #113 are more than both (2) and (3) so we should probably answer that question independently of this issue.

@ptomato @12wrigja FYI

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions