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

Use "Symbol" for internal entity properties #135

Merged
merged 1 commit into from
Oct 20, 2021

Conversation

kettanaito
Copy link
Member

@kettanaito kettanaito commented Oct 19, 2021

GitHub

Changes

  • Entity’s internal properties are no longer set using the reserved __type and __primaryKey. Instead, it uses symbols:
    • Symbol('type')
    • Symbol('primaryKey')
  • __nodeId is removed from internal entity properties. It was a left-over from the previous implementation of relationships and is no longer used.
  • Removes the concept of "internal entity" and the related InternalEntity type. The entity is now the same internally and publicly, as the Symbols representing internal properties are non-enumerable.
  • Database events for “create” and “update” methods now serialize the internal properties of the entities they are given. Symbols are lost when transmitting data over the BroadcastChannel, so we need to preserve the transmitted entity’s internal properties in a manually attached regular property. The event listener then re-attaches the symbols to the received entity using the inheritInternalProperties function.
  • Tests adjusted to assert the existence of the symbols on the entities. Note: This is only relevant to Jest, which compares symbols when using the .toEqual assertion. The end-developer never sees nor operates with the internal symbols as they are non-enumerable.
  • Improves the type annotations for the Database class to restrict modelName values to the keys of the given dictionary.

Motivation

This change originates from #132, where a recursive relationship resulted in an infinite loop. Upon investigation, the loop originated from the removeInternalProperties function that stripped a given entity of its internal properties (those were stored as regular properties under reserved keys). That function eventually stumbled upon a relational property, read it, tried to clean it, then found the nested relation, then nested, etc (the relation is expected to be circular but the library must not hang on it).

Diving into the matter further, I’ve tried storing the internal properties in a different way. My first try was to use Object.defineProperties, which appends properties that are non-enumerable by default. This worked nicely until I realized that the properties appended that way are lost when you assign the object (entity) into another data structure:

const entity = { id: 'abc-123' }
Object.defineProperty(entity, 'primaryKey', {
  // Exclude this property from being listed
  // when the end-developer works with the public entity.
  enumerable: true,
  value: 'id'
})
// Non-enumerable properties can still be accessed
// by reference.
entity.primaryKey // "id"

const db = new Map()
db.set('abc-123', entity)

const retrieved = db.get('abc-123')
// While regular properties are preserved,
retrieved.id // "abc-123"

// The defined ones are lost.
retrieved.primaryKey // undefined

This corrupted each entity once it was stored in the Database. Using Object.defineProperties was not an option for this use case.

Afterward, I’ve recalled symbols and thought to give them a try. It turned out symbols are great for this particular case, because they are non-enumerable, but also persist with the object regardless of where it’s being stored. It was decided to use symbols for storing internal entity properties.

Consequences

  • No need to account for internal properties whenever iterating over the entity (when updating, querying, etc.). They are not listed.
  • No need to remove internal properties from an entity before returning it to the end developer. Internal properties are stored in Symbols, which are non-enumerable.
  • Since Symbols are non-enumerable when the entity object is stringified to be sent over the BroadcastChannel in the sync extension (database operations synchronization between open tabs), it loses its symbols, corrupting the entity. I’d have to add a manual plain property called SERIALIZED_INTERNAL_PROPERTIES that temporarily stores respective properties, and then re-define them on the entity in another client.

@kettanaito kettanaito merged commit a20bf28 into master Oct 20, 2021
@kettanaito kettanaito deleted the refactor/internal-entity-props branch October 20, 2021 23:48
Copy link
Collaborator

@Aprillion Aprillion left a comment

Choose a reason for hiding this comment

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

👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support recursive relationships
2 participants