-
-
Notifications
You must be signed in to change notification settings - Fork 385
Gregsdennis/dynamic ref naming #1656
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
base: main
Are you sure you want to change the base?
Conversation
|
@gregsdennis, I actually started working on this a while ago (had to stop to prepare my talk for the conf). I should have communicated that I was work on it. Anyway, it's mostly the same (sometimes word for word) as what I have. The one thing I did different was that I want to move away from referring to dynamic anchors as fragments because fragments are a URI concept. Do you mind if push to this branch some of what I was working on to reflect that change? |
|
@jdesrosiers yeah feel free. I likely missed something anyway. |
| The value of the `$dynamicRef` property MUST be a string and MUST conform to the | ||
| plain name fragment identifier syntax defined in {{fragments}}.[^3] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no reason for this to be limited to fragment identifier syntax because it's not a fragment identifier anymore. Any objection to changing this to be any string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think keeping it this syntax simplifies schemas. Yeah it'll work with any string, but I don't think it helps necessarily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with not changing it, but I want to include a comment noting why it's restricted even though there's no technical reason it needs to be like there is with $anchor. Then I realized, I couldn't write that up because I didn't understand your reasoning 😅. So, can you help clarify,
I think keeping it this syntax simplifies schemas.
I'm not sure what you mean. What does it simplify exactly. Removing the restriction simplifies implementations that no longer need to make that check. It also simplifies writing schemas because the schema author doesn't need to know the rules for what's allowed and what isn't. It also simplifies the meta-schema. The only reasons I can think of for keeping it is to maintain consistency with $anchor or to just avoid changing it because the change doesn't add any value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm less concerned about it being anchor-like and more concerned with it being identifier-like. I think allowing any string (whitespace, special chars, etc.) could get confusing, but admittedly, that's probably the dev's responsibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, it's a style thing. Yeah, that's the schema author's concern. We shouldn't need to address that in the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name could be used as a function name via code generation, so a limited character set is useful in that case.
What kind of change does this PR introduce?
Update
Issue & Discussion References
$dynamicAnchorresource identification is confusing #1655Summary
Changes usages of
$dynamicRefto use the same plain-name identifier defined by$dynamicAnchorinstead of the IRI syntax.Does this PR introduce a breaking change?
Yes. The IRI syntax is no longers supported.
Implementations still have the option to deviate from the specification and support IRI-syntax anchors, but this must be disabled. (Do we need to explicitly state this?)