clean up $dynamic*

[gregsdennis]

What kind of change does this PR introduce?

Clarification

Issue & Discussion References

• Related to Clean-up of stable features and removal of unstable features #1444
• Replaces Drop the initial resolve step for dynamic references #1142

Summary

Cleans up the $dynamic* keywords.

Does this PR introduce a breaking change?

No.

[jviotti]

I left a few notes, but it reads well IMHO

[jdesrosiers]

I have some issues with the way the concepts in the changed sections are presented. However, I think those issues can be considered separately from this PR. I'll briefly describe that issues I have, but I'll consider them out of scope for this PR.

In scope for this PR:

• $anchor only applies to $ref and $dynamicAnchor only applies to $dynamicRef
• Remove the initial resolution step for dynamic references

I have one comment inline, but otherwise I think this does what it needs to do.

Issues that can be considered out of scope:

• There's conflation of the concept of a URI fragment and the identifier created by an anchor. Anchors don't create fragments, they create resource-local identifiers. Those identifiers can be used in a URI fragment to reference the location that local identifier identifies.
• The {#anchors} section describes anchors as a shorthand for a full IRI with a plain name fragment that's resolved against the schema resource. I find that to be an awkward and unnecessarily complex way of describing the concept.
• The spec still says in multiple places that $dynamicRef is primarily useful for extending recursive schemas. This hasn't been true since it was $recursiveRef. That should be cleaned up.
• I've always thought the way dynamic scope was described in the spec was confusing and it should be improved. The PR doesn't touch that section, but it does reference it.

[jdesrosiers]

I don't think this is clear enough. I read this as just a plain name identifier, not a URI. But the footnote describes it as a fragment-only URI with plain name fragment. It was part of my proposal that it should become just a plain name identifier because there's no need for it to be a URI without the initial resolution step, but I didn't think there was consensus on that. So, I'm not sure what you intended, but right now I'd say it's contradictory.

[gregsdennis]

the footnote describes it as a fragment-only URI with plain name fragment

The footnote says it has that syntax, not that it is an IRI fragment.

It was part of my proposal that it should become just a plain name identifier because there's no need for it to be a URI without the initial resolution step

I assume you're talking about using the plain name in $dynamicRef? If we don't use the IRI (I do have "URI" in the above text) fragment syntax in $dynamicRef, then it won't be backward compatible (if that matters to you here).

[jdesrosiers]

I assume you're talking about using the plain name in $dynamicRef?

Yes. Just the plain name not as part of a fragment-only IRI.

If we don't use the IRI (I do have "URI" in the above text) fragment syntax in $dynamicRef, then it won't be backward compatible (if that matters to you here).

Removing the initial resolution step already makes it a breaking change, so I'm not too worried about introducing another. However, I recognize that there's a difference in the impact of the two changes. I doubt the initial resolution is used anywhere other than the "links" schema (used by the hyper-schema meta-schema), so there's almost no impact. The other change is minor (just remove the leading #), but affects every use of $dynamicRef.

Personally, I think that using IRI syntax for something that isn't an IRI is unnecessary and confusing and makes for a bad experience for people trying to learn this feature. I think it's worth fixing while we still can.

[gregsdennis]

Let's keep it as is for now. If you can work up a voting issue, we can run it by the team.

[jdesrosiers]

Is there anything needed for this PR? I understand we have further changes coming, but this is a good step in the right direction.

I'm fine with leaving this value as a fragment-only IRI for now, but the text is still ambiguous at best about what the value of $dynamicRef should be. Right now it seems to say that it should be just a fragment identifier (not an IRI) and that contradicts the footnote that says it's an IRI. All other concerns I mentioned can be addressed later, but I think the spec should at least be self-consistent before we merge.

[gregsdennis]

What if we say $dynamicRef SHOULD be just the same name but MAY be formatted as a plain name fragment and that implementations MUST support both?

This kind of gets us into the same situation that we had when we started to forbid fragments in $id, but I think it's a good way to start the transition toward using the plain name. I don't think this would be that great of a burden on implementations.

[gregsdennis]

Suggestion:

The value of the $dynamicRef property MAY either be

• the exact string value as found in the corresponding $dynamicAnchor, or
• the $dynamicAnchor value formatted as a valid plain name fragment.

Implementations MUST support both variations.¹

Footnotes

1. Since $dynamicRef/$dynamicAnchor references are a closed system, distinct from $ref/$id/$anchor references, it is not necessary for $dynamicRef to use IRIs or their syntax. However, due to the large number of existing 2020-12 schemas which currently use an IRI fragment syntax in $dynamicRef, support for this formatting is still required. It is likely that this will eventually be phased out. ↩

[karenetheridge]

Supporting two different syntaxes at the same time seems really confusing. Plus, making this change means that it will no longer be possible to use a dynamicRef to another document or even a different namespace in the same document, which seems like a huge step backwards.

This PR is labelled "Does this PR introduce a breaking change? No", which seems false now.

[gregsdennis]

Supporting two different syntaxes at the same time seems really confusing. - @karenetheridge

I agree, but I don't see another incremental way toward abandoning the IRI format for $dynamicRef, which seems to make sense to do.

making this change means that it will no longer be possible to use a dynamicRef to another document or even a different namespace in the same document

I think you may have missed that $dynamicRef can no longer operate as $ref. It can only target $dynamicAnchor moving forward. This doesn't have any impact on its ability to reference $dynamicAnchor across documents/resources.

[jdesrosiers]

What if we say $dynamicRef SHOULD be just the same name but MAY be formatted as a plain name fragment and that implementations MUST support both?

I think this is a great idea in normal circumstances, but considering where we are with the transition to the stable spec, I don't think this is the right approach. Once we do this release, we're committing to no breaking changes. We decided on a process for introducing breaking changes, but we should be doing everything we can to avoid having to use that process. Releasing the spec with a something that we already know needs to change is not ideal. I think we need to either change it and make it stable, or allow both and make it experimental until we're comfortable removing the deprecated syntax. I'm ok with either.

• the $dynamicAnchor value formatted as a valid plain name fragment.

I don't think I've communicated well enough my concern about the term "plain name fragment" because you keep using that term the same way and I understand it to mean something different than you do. I understand this term to mean the fragment part of a URI that uses a plain name identifier. You clearly understand this to mean a fragment-only URI using a plain name identifier. This language needs to be more precise to avoid the ambiguity.

However, due to the large number of existing 2020-12 schemas which currently use an IRI fragment syntax in $dynamicRef,

I don't think it's accurate to say there are a large number of schemas using $dynamicRef. We don't know for sure, but I think dynamic references are a very rarely used feature. I think only a small number of power users understand how to use it.

making this change means that it will no longer be possible to use a dynamicRef to another document or even a different namespace in the same document

You can still express all of the same constraints you could before. Just the way you do it is a little different. The initial resolution step was only necessary because of the bookending rule. Because the bookending rule was removed, the initial resolution step became unnecessary. See #1140 for the discussion.

This PR is labelled "Does this PR introduce a breaking change? No", which seems false now.

Removing the initial resolution step is already a breaking change. That label is wrong regardless of whether we change $dynamicRef to not be a URI.

[jdesrosiers]

Found one more issue. Line 550 talks about the "IRI resolution effects" of $dynamicRef. This PR removes the IRI resolution step from dynamic references, so that statement doesn't make sense anymore.

[gregsdennis]

Anchors don't create fragments, they create resource-local identifiers.

The text is:

$anchor defines a reference target for $ref. The fragment defined by this keyword is appended to the IRI of the schema resource containing it.

I read this as trying to say something that's not incorrect, but I agree this is clunky.

The {#anchors} section describes anchors as a shorthand for a full IRI with a plain name fragment that's resolved against the schema resource.

I can't find "shorthand" anywhere in the new or the old text. Can you be more specific where the text you're referencing is?

The spec still says in multiple places that $dynamicRef is primarily useful for extending recursive schemas. This hasn't been true since it was $recursiveRef.

I added the word "primarily" as we've since found other use cases. I left it in because this is how it's used in the meta-schemas. I'm open to alternatives or just removing the use case.

I've always thought the way dynamic scope was described in the spec was confusing and it should be improved.

I'm pretty sure I tried to edit this section recently. I think I had started rewriting some of it but was having trouble, so I just left it.

[gregsdennis]

The build on main must have been failing, too...

[jdesrosiers]

Anchors don't create fragments, they create resource-local identifiers.

The text is:

$anchor defines a reference target for $ref. The fragment defined by this keyword is appended to the IRI of the schema resource containing it.

I read this as trying to say something that's not incorrect, but I agree this is clunky.

I don't think, "The fragment defined by the keyword" is correct. The keyword doesn't define the fragment. It defines an identifier for the fragment.

{
  "$defs": {
    "foo": {
      "$anchor": "foo",
      "type": "string"
    }
  }
}

Here, the fragment is { "$anchor": "foo", "type": "string" } and the identifier for the fragment is foo.

So, the minimal thing we can do to fix this is to change "fragment" to "fragment identifier". That also provides consistency with the `{#fragments} section which uses that term, which it gets from the W3C recommendation we link to.

I can't find "shorthand" anywhere in the new or the old text.

I didn't mean that it literally used the word "shorthand". I meant that was how it described it. The following is what I'm referring to.

The fragment defined by this keyword is appended to the IRI of the schema resource containing it. As discussed in {{id-keyword}}, this is either the nearest $id in the same or an ancestor schema object, or the base IRI for the document as determined according to RFC 3987 and RFC 3986.

I read this as saying that "$anchor": "foo" is shorthand for the IRI https://example.com/myschema#foo. I can see how someone could think about it that way, but resource resolution and fragment resolution are two separate procedures and it seems awkward to me to smash them together. This section should be describing how to mint a fragment identifier. It doesn't need to get into how to use them in a URI. That's all standard stuff that we don't need to repeat.

I've taken a stab at rewriting that section that I'll follow up this PR with. So, maybe don't worry too much about this section for this PR.

I've always thought the way dynamic scope was described in the spec was confusing and it should be improved.

I'm pretty sure I tried to edit this section recently. I think I had started rewriting some of it but was having trouble, so I just left it.

Yeah, as much as I dislike that section, I recognize that it's a very difficult concept to explain!

[jdesrosiers]

The build on main must have been failing, too...

Yeah, it looks like the PR that added the linting was behind main when it was merged. So the PR was passing, but it started failing once it was merged.

[gregsdennis]

@jdesrosiers Is there anything needed for this PR? I understand we have further changes coming, but this is a good step in the right direction.

[karenetheridge]

This is a different ADR and shouldn't be changing in this PR?

[gregsdennis]

There were some formatting things that were merged and failing on the main branch and causing this build to fail. Jason and I just cleaned it up here.