Ignore dfns that have an invalid type #659
Conversation
This update restricts dfns extraction to definitions that have a valid type, as defined in: https://tabatkins.github.io/bikeshed/#dfn-types (+ `namespace` and `event` that are valid but do not yet appear in the doc) In practice, the only invalid type that appears in Editor's Drafts is `idl`, which ReSpec uses to flag internal slots. To avoid losing definitions while ReSpec gets fixed and while the changes propagate to all specs and /TR versions of the spec, the code automatically converts `idl` definition types to `attribute`. Data validation is something that we would typically do at a later stage, through anomaly reports, or through patches as we produce packages. That said, dfns are more time sensitive than other types of data that Reffy extracts from the specs and the validation process would need to be semi-manual. Dfns with an invalid type get reported as warnings to the console. Fixes #658.
src/browserlib/extract-dfns.mjs
Outdated
| // propagates to all EDs and /TR specs. To be dropped once crawls no | ||
| // longer produce warnings. | ||
| if (node.getAttribute('data-dfn-type') === 'idl') { | ||
| node.setAttribute('data-dfn-type', 'attribute'); |
There was a problem hiding this comment.
I've commented in https://github.com/w3c/respec/issues/3644#issuecomment-864894696 - I'm not convinced attribute is the right approach.
Code looks good otherwise.
There was a problem hiding this comment.
We're still discussing whether it would be worth creating an additional type in speced/spec-dfn-contract#1.
In the meantime, w3c/respec#3644 was closed with ReSpec now flagging internal slots with an attribute type and internal methods with a method type. I've updated the code to the same here and propose to go ahead with this while we converge on new types (or on making these dfns private by default one way or another).
| } | ||
| return node; | ||
| }) | ||
| .filter(hasValidType) |
There was a problem hiding this comment.
Thinking some more - instead of removing definitions with an invalid type, shouldn't we default them back to dfn?
There was a problem hiding this comment.
I'm hesitant to do that. The good thing about removing definitions with an invalid type is that no one can reference them. When someone needs to do that, they will notice that the dfn is missing and raise an issue somewhere to have that fixed, and this should eventually appear on our radar.
If we default these definitions to dfn, people may just ask for a data-export attribute to be added, and then they can start to link to it, probably using a wrong shorthand syntac. We may not notice the issue until both the source spec that defines the term and the spec that references it are wrong.
Obviously, we could notice if we had a proper way to deal with the "warnings" that Reffy logs. I'm not utterly confident that we'll look into the workflow logs, and it's more work to have a better alert mechanism. Useful work but more work... Plus, if we do that, we can just as well remove the dfns: we would have them fixed in the source spec.
There was a problem hiding this comment.
ok, I can see that making kind of sense; the risks are probably limited in any case if ReSpec starts flagging unknown definition types the same way bikeshed does.
As done in ReSpec, internal slots dfns get flagged with an `attribute` type, while internal methods dfns get flagged with a `method` type.
This update restricts dfns extraction to definitions that have a valid type, as defined in:
https://tabatkins.github.io/bikeshed/#dfn-types
(+
namespaceandeventthat are valid but do not yet appear in the doc)In practice, the only invalid type that appears in Editor's Drafts is
idl, which ReSpec uses to flag internal slots. To avoid losing definitions while ReSpec gets fixed and while the changes propagate to all specs and /TR versions of the spec, the code automatically convertsidldefinition types toattribute.Data validation is something that we would typically do at a later stage, through anomaly reports, or through patches as we produce packages. That said, dfns are more time sensitive than other types of data that Reffy extracts from the specs and the validation process would need to be semi-manual.
Dfns with an invalid type get reported as warnings to the console.
Fixes #658.