Add optional direction for literals #38
Conversation
🦋 Changeset detectedLatest commit: 14f33a2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
data-model.d.ts
Outdated
|
|
||
| export interface DirectionalLanguage { | ||
| language: string; | ||
| direction?: 'ltr' | 'rtl' | ''; |
There was a problem hiding this comment.
In what cases would '' be used as input over undefined
There was a problem hiding this comment.
Because direction can be a falsy value, as discussed in the PR here: rdfjs/data-model-spec#175
That actually reminds me, we should also allow null here...
rubensworks
force-pushed
the
feature/direction
branch
from
951da64 to
14f33a2
Compare
|
This probably is something we should also (like #6) first discuss in the spec itself? |
Yes, we have rdfjs/data-model-spec#172 and rdfjs/data-model-spec#175 for this. But in any case, I think we'll want to put this on hold until w3c/rdf-concepts#79 is resolved. |
|
@tpluscode Do you agree with merging this one? It has been open for quite a while now. |
|
And what about waiting for w3c/rdf-concepts#79? |
|
Ah, I read into the resolution and it actually appears that this feature is not planned for RDF? |
|
From the discussion, I thought w3c/rdf-concepts#79 was resolved. |
It is in fact going to be included (https://w3c.github.io/rdf-concepts/spec/#dfn-dir-lang-string). |
| /** | ||
| * the direction of the language-tagged string. | ||
| */ | ||
| direction?: 'ltr' | 'rtl' | '' | null; |
There was a problem hiding this comment.
JS noob question: The specification states direction?: DOMString. Does it allow null?
There was a problem hiding this comment.
I'm not too sure about the semantics of direction?: DOMString in ReSpec.
But at least the text in rdfjs/data-model-spec#175 says where <code>direction</code> is optional or can be falsy.
Falsy includes null and undefined in JavaScript.
There was a problem hiding this comment.
Good point! Should we ensure that the WebIDL definition is synchronized with the specification text before merging both MRs?
There was a problem hiding this comment.
null and undefined are both implicitly included in other parts of the spec, such as Term.equals().
So this would probably be part of a broader change.
Might be good to open a separate issue for this?
|
@tpluscode The issue has been resolved. Do you agree with merging this PR here? |
|
Ok, let's go |
This adds typing support for base directions in literals, as discussed in rdfjs/data-model-spec#175