Skip to content

Full Refract Serialisation#36

Merged
kylef merged 2 commits intomasterfrom
kylef/full-serialisation
Apr 4, 2017
Merged

Full Refract Serialisation#36
kylef merged 2 commits intomasterfrom
kylef/full-serialisation

Conversation

@kylef
Copy link
Member

@kylef kylef commented Mar 27, 2017

This pull request aims to resolve some ambiguities in the current informal serialisation along with pave the way to having a full clear specification for full Refract serialisation which currently is not defined.

Many implementations do not support both refracted and un-refracted JSON data in many places leading to Refract tooling that is coupled to JSON serialisation. This PR attempts to resolve these problems by:

  1. Making serialisation clearer in a specification.
  2. Removing possibilities that certain elements can be serialised differently based on the tooling.
  3. Prevent serialisation ambiguities.

This would supersede #17.

@kylef kylef force-pushed the kylef/full-serialisation branch from dcb8995 to adcb72d Compare March 27, 2017 10:03

The content inside JSON Serialisation MUST always be a Refracted element, an
array of Refracted element or a primitive type such as string, number, boolean
or none. The only exceptions to this rule is for any element types described
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why there is a need for the last sentence here.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are exceptions to the rule I outlined as member stores a JSON object directly as content instead. Any element in the core Refract specification can provide exceptions to these rules.

{
  "element": "member",
  "content": {
    "key": {
      "element": "string",
      "content": "id"
    },
    "value": {
      "element": "string",
      "content": "Hello"
    }
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

One thing that could be done here is to make the content of the "Member Element" to be it's own definition (maybe a Key Value Object or something). Then you can be more specific to say it MUST be:

  1. Refracted element
  2. Array of refracted elements
  3. Values (string, number, boolean)
  4. Key Value Object
  5. None

Just a thought. Is there any other value from the spec?

Copy link
Contributor

Choose a reason for hiding this comment

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

Element Pointer and Link Element have direct objects.

Copy link
Member Author

@kylef kylef Mar 27, 2017

Choose a reason for hiding this comment

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

I do not think Link Element actually would be exclusions to this rule.

Link Element does not use content and does not have direct content at all. It only supports href and relation as ATTRIBUTES not inside content.

** Element Pointer** on the other hand does use an enum where the value may be an object, but I wonder if we can simplify the element pointer value to be a string element with an attribute instead.

Currently the following is defined in the spec for the value for Element Pointer:

Members

  • (string) - A URL to an ID of an element in the current document
  • (object) - The URL and path of the referenced element
    • href (string, required) - URL or ID of element
    • path (enum) - Path of referenced element to transclude instead of element itself
      • meta - The meta data of the referenced element
      • attributes - The attributes of the referenced element
      • content - The content of the referenced element

Perhaps this could be simplified to an element:

+ attributes
    + path (enum, optional)
        + meta - The meta data of the referenced element
        + attributes - The attributes of the referenced element
        + content - The content of the referenced element
+ content (string)

That way, the only "special-cased" is member, which we can say the value is a "key-value pair" as @smizell suggests.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kylef I like that. Are you using that anywhere that would be broken?

Also, you may want to define another value for path to say the pointer points to either:

  1. The entire element
  2. The meta of the element
  3. The attributes of the element
  4. The content of the element

Copy link
Member Author

Choose a reason for hiding this comment

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

I've moved Element Pointer to a separate PR: #38

I'm not sure on what It may break, but since we control most of the tooling that produces and consumes Refract/API Elements it is a better time than any to simplify and improve the specification for any wider consumption.

For example, it is NOT possible to serialise an arbitrary object inside the
content value for elements not defined within the base Refract specification.
Otherwise it can be ambiguous for consumers whether the JSON object is another
element or JSON data.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a note here, we put a couple of rules in the spec to make this possible I think (unless I'm missing something).

  1. The element property is required for all Refract elements.
  2. The element property is reserved in a Refract document

I don't think we've worked through this a lot, but it seems it may allow for arbitrary objects or arrays to be within the content, meta, or attributes while ensuring a serializer/deserializer can make decisions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, reserving element keyword will help fix this issue. (And we did). But IMO we want the serialisation rules to be more stricter and thus if content is an object, we want it to be fully serialized as object element.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, I can't find the line about the second point. I'm thinking maybe we didn't put it in the spec and chose to use it as a rule in the libraries. Maybe it should make it in the spec to make some of this easier?

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, not saying it should go into the main spec, but agree there should be some serialization/deserialization spec and maybe this rule should go there. If it does, it may make for a simpler set of rules.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry @pksunkara, I had to refresh to see your comment. I see what you're saying—makes sense to me.

@kylef kylef force-pushed the kylef/full-serialisation branch from adcb72d to 0a3dc34 Compare April 4, 2017 14:40
@kylef kylef merged commit 77ba283 into master Apr 4, 2017
@kylef kylef deleted the kylef/full-serialisation branch April 4, 2017 14:40
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.

3 participants