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

Explain layering of the Store level of abstraction on the Marshal level #3567

Closed
wants to merge 2 commits into from

Conversation

erights
Copy link
Member

@erights erights commented Jul 31, 2021

No description provided.

@erights erights self-assigned this Jul 31, 2021
@erights erights force-pushed the doc-store-marshal-layering branch 2 times, most recently from 9373bb9 to cb43110 Compare August 1, 2021 05:58
The **data styles** tag data-like containers. If a data-like container contains only data, then it is only data, passing the `isOnlyData` type test.
* A *copyArray* is a simple JavaScript array containing only passables.
* A *copyRecord* is a simple JavaScript record-like object, with only string-named own data properties containing only passables.
* A *copyTagged* is the extensibility point used by the Store level to implement *copySets* and *copyMaps*.
Copy link
Contributor

Choose a reason for hiding this comment

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

copyTagged needs more explanation. You provide various bits of precise definitional language but never say what it actually is in a more informal sense that people can actually understand. That renders the narrative much harder to follow, when I expect (but am not sure) that it's actually fairly simple and straightforward.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. Being very precise can actually make it harder for readers to understand, when the precision requires new terms and concepts. Being a little more imprecise and using analogies to orient the reader might be helpful.

Copy link
Contributor

@JimLarson JimLarson left a comment

Choose a reason for hiding this comment

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

Even after reading the README.md one level up, I still don't have enough context to give meaningful approval. As a reader, I give some suggestions, but feel free to reject them.

packages/marshal/docs/marshal-vs-store-level.md Outdated Show resolved Hide resolved
## The Store level organized by `keyStyleOf`

**`keyStyleOf(p: Passable) => KeyStyle`**<br>
where `KeyStyle` is an enumeration quite similar to `PassStyle`
Copy link
Contributor

Choose a reason for hiding this comment

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

Say briefly why you need a new enumeration for keys.

**`keyStyleOf(p: Passable) => KeyStyle`**<br>
where `KeyStyle` is an enumeration quite similar to `PassStyle`

`KeyStyle`:
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of re-creating a similar hierarchy, consider explaining just the differences:

  • The following are identical: primitive styles ("undefined", "null", "boolean", "bigint", "number", "string", "symbol"), "copyArray", "copyRecord", "remotable".
  • The following are disallowed and will throw if passed to keyStyleOf(): errors, promises, and metaTagged.
  • Data with a passStyle of "copyTagged" must have a valid representation, and will have a keyStyle of "copySet" or "copyMap" depending on that representation.

* Store's **"remotable"** is identical to Marshal's. Every remotable is a key. On two remotables `p` and `q`, `sameKey(p,q)` compares using the remotable's unique unforgeable identity. Thus, for two remotables, `sameKey(p,q)` iff `sameStructure(p,q)` iff `p === q` iff `Object.is(p,q)` iff `sameValueZero(p,q)`.
* The remaining passables: errors, promises, and metaTaggeds, are not keys. `keyStyleOf` applied to any of them throws.

Primitives and remotables are keys. copyArrays, copyRecords, copySets, and copyMaps are key-like containers. If a key-like container contains only keys, then it is a key.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe have this paragraph before explaining keyStyle.

Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

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

A handful of questions, but seems like a good taxonomy and a basis for discussion.

packages/marshal/docs/marshal-vs-store-level.md Outdated Show resolved Hide resolved

Primitives and remotables are keys. copyArrays, copyRecords, copySets, and copyMaps are key-like containers. If a key-like container contains only keys, then it is a key.
* A *copySet* can only contain keys, and so each copySet is necessarily a key.
* A *copyMap* entry can use only use a key as a key, but can use any passable as a value including unrecognized copyTaggeds. But a copyMap is a key only of all of its values are keys.
Copy link
Member

Choose a reason for hiding this comment

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

You said above that everything (including copyMap) must be hardened to qualify as a passable, and therefore as a key. But that doesn't make the Map contents immutable. Does the copyMap's identity (as a key) remain stable in the face of additions/removals/changes to the copyMap contents?

Copy link
Member

Choose a reason for hiding this comment

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

Also, does sameKey on a copyMap require a full examination of the contents every time? I imagine you have some clever trick up your sleeve, but using any container as a key (in a language that lacks hashable object identities) sounds pretty expensive.

@erights erights marked this pull request as draft August 6, 2021 20:27
@erights
Copy link
Member Author

erights commented Aug 6, 2021

Converted to Draft until it is more coherent.

@erights erights changed the base branch from master to markm-new-passable-styles August 7, 2021 19:12
@erights erights force-pushed the markm-new-passable-styles branch 5 times, most recently from cb6d43f to a87adc6 Compare August 10, 2021 01:26
@erights
Copy link
Member Author

erights commented Aug 23, 2021

Closing in favor of #3653

@erights erights closed this Aug 23, 2021
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.

5 participants