Skip to content

RFC to reserve keyword element.#29

Merged
pksunkara merged 3 commits intomasterfrom
smizell/reserve-element
Dec 15, 2015
Merged

RFC to reserve keyword element.#29
pksunkara merged 3 commits intomasterfrom
smizell/reserve-element

Conversation

@smizell
Copy link
Contributor

@smizell smizell commented Dec 3, 2015

No description provided.

@danielgtaylor
Copy link
Contributor

Love it 👍. This basically fixes #17 by formally making element reserved for serialized full refract.

This means that no user can ever use the keyword element.

Actually, they can as long as the serializer is smart enough to refract that object into something like:

{
  "element": "object",
  "content": [
    {
      "element": "member",
      "content": {
        "key": "element",
        "value": "..."
      }
    }
  ]
}

It does make writing by hand and using that particular key more difficult though 😄

@smizell
Copy link
Contributor Author

smizell commented Dec 3, 2015

Actually, they can as long as the serializer is smart enough to refract that object

You know, that is true. For instances of Object Element, if there is an "element" member, the entire object should be refracted. As for parsing, they would just do the same as you've mentioned. That seems easy to get around when serializing.

Copy link
Contributor

Choose a reason for hiding this comment

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

...things that need to be refracted when serializing.

We may want to define here exactly what that means. For example, we must refract member elements or expect implementations to handle them some other way (in object's content or in attributes like hrefVariables for example because it reintroduces domain-specific knowledge). Maybe a whitelist of types supported to not be refracted? Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So in thinking of this, I don't think we should pursue this too far. But it opens to lots of possibilities. Take this for example:

{
  foo: 'bar',
  baz: 'bab'
}

This could be considered valid Refract if we reserve element. Now what if the "bar" value is some non-primitive element?

{
  foo: {
    element: 'non-primitive',
    content: 'bar'
  },
  baz: 'bab'
}

The only reason we would need to refract the entire object and use member elements here is if foo or baz have non-primitive elements or have meta or attributes.


My concern here is interop with current tooling. But in allowing us to optionally Refract things, it does make things a lot cleaner :)


we must refract member elements

Given the example above, I'm not sure we have to, though interop with cooling tooling could require it. We would only need member elements if (a) the member element has meta or attributes or (b) a key in the object is non-primitive or has meta or attributes. Do you think this would still require a "MUST refract" here?

@zdne
Copy link
Member

zdne commented Dec 4, 2015

Let's do it

Copy link
Contributor

Choose a reason for hiding this comment

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

anywhere the element key is present, right? Calling it keyword sounds weird and allows ambiguity of it being present in a value of an object.

@pksunkara
Copy link
Contributor

Minor comments, otherwise good to go.

@smizell smizell mentioned this pull request Dec 15, 2015
@smizell
Copy link
Contributor Author

smizell commented Dec 15, 2015

@pksunkara comments addressed

Copy link
Contributor

Choose a reason for hiding this comment

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

s/property/keyword

pksunkara added a commit that referenced this pull request Dec 15, 2015
@pksunkara pksunkara merged commit 8e0d75a into master Dec 15, 2015
@pksunkara pksunkara deleted the smizell/reserve-element branch December 15, 2015 20:00
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.

4 participants