Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(api) refactor api for ergonomics and type safety #703

Merged
merged 11 commits into from
Mar 6, 2024

Conversation

dselman
Copy link
Sponsor Contributor

@dselman dselman commented Sep 2, 2023

Refactors the API.

The goals are:

  1. To better align with the metamodel in terms of naming. We currently have things like RelationshipDeclaration and EnumValueDeclaration that are not declarations.
  2. To improve type-safety, particularly around type introspection. The new ModelElement root class holds all the introspection methods, such as isConcept / isProperty etc. so we should be able to banish the error prone pattern of writing thing.isProperty?.() which is proliferating. This should also allow TypeScript code to refer to ModelElement in most cases, rather than ClassDeclaration | Property.
  3. Reduce duplication and confusion between the different types of declaration we now have: concepts, enums, scalars, maps.
  4. Reduce breaking changes as far as practicable.
  5. Allow visitors to more easily and consistently switch over the meta-type so we can banish the use of instanceof in the visitors (and elsewhere)

Notes:

  • Everything in a ModelFile is a ModelElement. This becomes our single place to put type introspection utilities.
  • All Declarations (top-level named things in the Model File) extend Declaration
  • Anything that can have a decorator applied is an instance of Decorated, except for ModelFile which exposes decorators but is itself not a Decorated (because we don't want a ModelFile to be a ModelElement).
  • Things in a ModelFile that have properties are a ClassDeclaration. I.e. Maps and Scalars are Declarations, but they are not ClassDeclarations. An EnumDeclaration is a ClassDeclaration, because we consider EnumValues to be properties.
  • RelationshipDeclaration is renamed to RelationshipProperty
  • EnumValueDeclaration is renamed to EnumValue
  • Decorator is now also a ModelElement
  • Lots of changes for map serialisation to align with what we do for Fields (and removes lots of code). This includes storing DateTimes in map keys and values as dayjs instances for consistency with other DateTime fields. I think this will fix some bugs. E.g. Map does not support polymorphic values #769 and ensures type conversion to/from JSON is consistent.

Changes

  • Adds a ModelElement root class to hold all meta-type introspection methods
  • Refactor Decorated to extend ModelElement
  • RelationshipDeclaration class renamed to RelationshipProperty
  • EnumValueDeclaration class renamed to EnumValue

Flags

Screenshots or Video

Related Issues

  • Issue #
  • Pull Request #

Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Vital features and changes captured in unit and/or integration tests
  • Commits messages follow AP format
  • Extend the documentation, if necessary
  • Merging to main from fork:branchname

Signed-off-by: Dan Selman <danscode@selman.org>
Signed-off-by: Dan Selman <danscode@selman.org>
@mttrbrts mttrbrts mentioned this pull request Sep 4, 2023
@mttrbrts
Copy link
Sponsor Member

mttrbrts commented Sep 4, 2023

How closely does this mirror the metamodel definition now?

@mttrbrts mttrbrts changed the base branch from main to v4.0.0 September 4, 2023 14:48
@mttrbrts mttrbrts added this to the v4.0 milestone Nov 8, 2023
mttrbrts and others added 6 commits November 20, 2023 12:44
Signed-off-by: Matt Roberts <code@rbrts.uk>
Signed-off-by: Matt Roberts <code@rbrts.uk>
Signed-off-by: Dan Selman <danscode@selman.org>
Signed-off-by: Dan Selman <danscode@selman.org>
Signed-off-by: Dan Selman <danscode@selman.org>
Signed-off-by: Dan Selman <danscode@selman.org>
@dselman
Copy link
Sponsor Contributor Author

dselman commented Jan 5, 2024

Tests pass. Now need to look at the code coverage.

Copy link
Sponsor Member

@mttrbrts mttrbrts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😌

Signed-off-by: Dan Selman <danscode@selman.org>
Signed-off-by: Dan Selman <danscode@selman.org>
@dselman dselman marked this pull request as ready for review February 28, 2024 14:02
@mttrbrts mttrbrts merged commit 6b696c7 into v4.0.0 Mar 6, 2024
9 checks passed
@mttrbrts mttrbrts deleted the ds-api-refactor branch March 6, 2024 11:23
@mttrbrts mttrbrts mentioned this pull request Mar 6, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants