Skip to content

Does @sealed consider order? #153

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

Closed
rubensworks opened this issue Mar 28, 2019 · 6 comments
Closed

Does @sealed consider order? #153

rubensworks opened this issue Mar 28, 2019 · 6 comments

Comments

@rubensworks
Copy link
Member

Due to the definition of @sealed, the following context will throw an error:

{
  "@context": [
    {
      "@version": 1.1,
      "Person": "http://schema.org/Person",
      "knows": "http://schema.org/knows",
      "name": {
        "@id": "http://schema.org/name",
        "@sealed": true
      }
    },
    {
      "name": "this_attempt_to_override_name_will_fail"
    }
  ]
}

As far as I can see, the spec does not seem to mention what should happen if sealing is done within a context that extends an existing context. Concretely, should the following also error?

{
  "@context": [
    {
      "name": "this_attempt_to_override_name_will_fail"
    },
    {
      "@version": 1.1,
      "Person": "http://schema.org/Person",
      "knows": "http://schema.org/knows",
      "name": {
        "@id": "http://schema.org/name",
        "@sealed": true
      }
    }
  ]
}

Two things are possible here:

  1. No error: The sealed scope only starts from the moment @sealed is used. This would allow modifications of term definitions up to this point, but not later on. (May be useful when combining multiple external contexts, and sealing afterwards)
  2. Error: The sealed scope propagates backwards from the moment @sealed is used. This may be useful for cases where context extensions need to be handled very strictly.

I have a slight preference for option 1. But either way, I think this should be clarified in the spec.

@pchampin
Copy link
Contributor

pchampin commented Mar 28, 2019

It is indeed option 1, and I don't understand what makes you think that sealing might propagate backward. First, overriding terms (sealed or not) in contexts has always been defined as order dependant. Second, the spec reads:

It prevents further contexts to override this term definition.

The emphasis on "further" is not in the original text, but it seems explicit enough to me.

edited: I originally wrote "option 2", while I meant "option 1", as pointed out by @gkellogg below -- thanks to him

@gkellogg
Copy link
Member

Actually, I think you mean option 1. In the second example, the first context defines "name" and the second context also defines "name" and seals it. Note that the second context does not update the definition of "name" but replaces it. So, there is No error, as the new definition marks itself with @sealed and it does not bear on the fact that a term with the same key was previously defined.

@dlongley
Copy link
Contributor

Yes to what @gkellogg said.

@pchampin
Copy link
Contributor

Yes of course, I meant "option 1". I edited my comment. Thx

@azaroth42
Copy link
Contributor

@iherman
Copy link
Member

iherman commented Mar 29, 2019

This issue was discussed in a meeting.

  • RESOLVED: Close #153, no issue, no editorial change needed in the spec.
View the transcript 4.2. Sealing and Order (propose close, no issue)
Rob Sanderson: Link: #153
rubenworks: I was reading the new 1.1 features, including sealing (now renamed protected)
… the example shows a property defined as sealed, then a second definition fails to override it
… but it was not entirely clear to me whether this worked when reversing the order of the definitions
… but pchampin pointed out that the spec reads “prevent further redefinitions”
… may be it would still be good to add an example to make it clearer
Dave Longley: +1 editorial, we’re in agreement
Rob Sanderson: it is mostly editorial
Gregg Kellogg: I think there are already many examples
… maybe a test case would be better here
Ivan Herman: this could be mentioned in the Best Practices document
… I’m always the guy who asks for more examples, but in this case I agree with gkellogg
Proposed resolution: Close #153, no issue, no editorial change needed in the spec. (Rob Sanderson)
Rob Sanderson: +1
Ruben Taelman: +1
Pierre-Antoine Champin: +1
Gregg Kellogg: +1
Dave Longley: +1
Tim Cole: +1
David I. Lehn: +1
David Newbury: +1
Ivan Herman: +1
Resolution #2: Close #153, no issue, no editorial change needed in the spec.

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

No branches or pull requests

6 participants