search

w3c / aria-common

Shared files for the ARIA repositories
Other
8 stars 15 forks source link

definitionMap not defined #98

Open jnurthen opened 4 years ago

jnurthen commented 4 years ago

https://github.com/w3c/aria/blob/cf85993c8f1dd89492028db8825b0893e91bb9e5/common/script/resolveReferences.js#L265

jnurthen commented 4 years ago

possibly caused by https://github.com/w3c/respec/pull/2682

marcoscaceres commented 4 years ago

@jnurthen, I wonder if we can work towards removing the some of the custom JS stuff added by the Aria spec? Unfortunately, it's using ReSpec in ways it was not really intended and not really supported anymore.

marcoscaceres commented 4 years ago

It seems like we could remove all event dependencies that are for "end-all", and run any functions needed with postProcess.

jnurthen commented 4 years ago

I would love to remove as much of the custom JS as possible. @halindrome wrote most of it though so as he is no longer involved we are not always 100% clear on what all of it actually does. We generally only investigate it when something goes wrong.

marcoscaceres commented 4 years ago

I'll just add that anything on the ReSpec config object not added by the end-user should be considered private. This includes the definitionMap.

marcoscaceres commented 4 years ago

Yeah, I'm also unsure as a lot of the code is not documented :(

jnurthen commented 4 years ago

just fyi all the webpayments and JSON-LD specs have the same code as the ARIA specs in this area.

marcoscaceres commented 4 years ago

Web Payments don't anymore :) I'm the editor for those. But yes, JSON-LD has some of these too but I think they are not dependent.

marcoscaceres commented 4 years ago

Anyway, I'll send some incremental PRs. We can tackle this in small pieces.

marcoscaceres commented 4 years ago

Is aria-requirements.html still maintained? Looks kinda outdated and maybe it can go away?

marcoscaceres commented 4 years ago

biblio.js can probably be deleted too. That's just aliasing a bunch of stuff, which is not very good practice.

jnurthen commented 4 years ago

biblio.js is for other specs so their references don't break apparently. @joanmarie has been removing as much as possible and we do aim to get rid of it.

jnurthen commented 4 years ago

as far as I know aria-requirements can go away... @michael-n-cooper can you please confirm. I've certainly never seen it before!

jnurthen commented 4 years ago

@marcoscaceres thanks so much for your help btw.

marcoscaceres commented 4 years ago

Np @jnurthen... @michael-n-cooper, @joanmarie, my thinking right now is that we should:

  1. remove aria.js - half of that doesn't seem to work anyway.
  2. fold acknowledgements/*.html files into acknowledgements.html (recursive includes is not really supported - so no one is showing up right now)?
  3. remove resolveReferences.js
  4. remove biblio.json - and use actual references and not aliases.
  5. Bring in the terms that are actually defined/used from common/terms.html into the spec. Having a common terms is duplication (bad!): a spec should only define its own terms and link to other specs for reference to other.

WDYT?

If there is something specific that should be built into ReSpec that the community needs, then we can look at adding that to ReSpec (we can try to get some additional funding or use what funding we have in the open collective to fund development if it will benefit other specs) - but the current model for how things are being done here is not sustainable both for ReSpec and for the Aria specs.

jnurthen commented 4 years ago
  1. aria.js contains a bunch of stuff that is necessary. Indeed I made some code changes in there a week or 2 ago. We use it to build up our states & properties and role information in the spec. It is necessary and most if not all of the code would need to be moved somewhere else if we considered removing it.
  2. either this or add the 3 includes directly in the specifications. I don't really have a preference on this.
  3. This is how we resolve the references amongst the specs which make up aria and allow us to reference specific versions of each of the specs. We would need to find another way to do this if we were to consider removing it.
  4. This is something we have been moving towards.
  5. Most of these terms are common amongst the specs of the suite which is why they are where they are. I'm not sure how we would work out which spec owned which term otherwise. Something you may not be aware of is that the common directory is maintained in the aria-common repository - and any changed there get copied into all of the aria repositories. So those terms aren't really duplicated they are defined just once.
marcoscaceres commented 4 years ago

This is how we resolve the references amongst the specs which make up aria and allow us to reference specific versions of each of the specs. We would need to find another way to do this if we were to consider removing it.

Could you give me a concrete example of what this is doing? I'm wondering why the "traditional" was was/is deficient?

marcoscaceres commented 4 years ago

Most of these terms are common amongst the specs of the suite which is why they are where they are. I'm not sure how we would work out which spec owned which term otherwise. Something you may not be aware of is that the common directory is maintained in the aria-common repository - and any changed there get copied into all of the aria repositories. So those terms aren't really duplicated they are defined just once.

Understood, but if they get included into other specs, then show up in other specs as <dfn>whatever</dfn> and so on. We now have the xref feature, along with really nice reference summary of what terms are defined where, to avoid having to include common definitions in this manner. This resolves a bunch of issues, and makes sure there is only "one source of truth" for a defined term.

jnurthen commented 4 years ago

Could you give me a concrete example of what this is doing? I'm wondering why the "traditional" was was/is deficient?

References within the suite should go to specific versions. For each of the specs we specify URLs so when a spec is an ED we can link to specific versions of the other suite members, we allow specific versions of the other specs to be referenced when in ED, WD, FPWD, CR and REC We also define roles (<rdef>), states (<sdef>) and properties(<pdef>) in the specs. These can be referenced in any of the other specs with <rref>, <sref> and <pref> and the code in resolve references takes care of pointing to the correct spec and the correct version of that spec.

jnurthen commented 4 years ago

Understood, but if they get included into other specs, then show up in other specs as <dfn>whatever</dfn> and so on. We now have the xref feature, along with really nice reference summary of what terms are defined where, to avoid having to include common definitions in this manner. This resolves a bunch of issues, and makes sure there is only "one source of truth" for a defined term.

I think we would be happy to try to fix these... Common definitions like this has been causing us pain - in as much as we have to split PRs up to make changes in multiple repos

marcoscaceres commented 4 years ago

References within the suite should go to specific versions. For each of the specs we specify URLs so when a spec is an ED we can link to specific versions of the other suite members, we allow specific versions of the other specs to be referenced when in ED, WD, FPWD, CR and REC

Has this worked? that seems confusing for implementers, developers, editors, etc.? (honestly don't know as I've very little experience with this spec, serious question)

jnurthen commented 4 years ago

Has this worked? that seems confusing for implementers, developers, editors, etc.? (honestly don't know as I've very little experience with this spec, serious question)

It is overkill in terms of all the versions but yes. Most times I simply set ED to 1 URL and all the others to something else. The only person who really has to do anything is the editor - no one else is even really aware of it - it just works.

pkra commented 1 year ago

@jnurthen transferring this from aria even though it's very stale. Is this still an issue?