-
Notifications
You must be signed in to change notification settings - Fork 146
editorial: [aria.js] generate roleInfo after respec runs #2769
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?
Changes from all commits
5e78491
89fe951
69df319
e4e2973
86912ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,9 @@ jobs: | |
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: "latest" | ||
| - run: npm i linkedom prettier@3.6.0 | ||
| - run: node ./common/script/buildRoleInfo.js > ./common/script/roleInfo.js | ||
| - run: npm i linkedom prettier@3.6.0 respec | ||
| - run: npx respec --src index.html --out index.html | ||
| - run: node ./common/script/buildRoleInfo.js | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not clear how this check would work. In PRs you don't have the respec esported spec, previews are built at run time.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh! You're right. My brain clearly stopped working at some point. I'll fix it.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @daniel-montalvo I've updated this to run respec before buildRoleInfo (and resetting index.html before diffing roleInfo.js) |
||
| - run: npx prettier --write --print-width 200 ./common/script/roleInfo.js | ||
| - run: git checkout index.html | ||
| - run: git diff --exit-code | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,74 @@ | ||
| import * as fs from "node:fs"; | ||
| import fs from "node:fs"; | ||
| import path from "node:path"; | ||
|
|
||
| import { parseHTML } from "linkedom"; | ||
| const __dirname = import.meta.dirname; | ||
|
|
||
| // NOTE: this script expects compiled respec output (e.g. gh-pages branch). | ||
| // If you are working on this file, you will need to run respec first. | ||
| // E.g. get a copy from gh-pages or run `$ npx respec --src index.html --out index.html` | ||
| const inputFilename = path.resolve('index.html'); | ||
|
daniel-montalvo marked this conversation as resolved.
|
||
| const outputFilename = path.join(__dirname, "roleInfo.js"); | ||
|
|
||
| const { document } = parseHTML(fs.readFileSync(inputFilename).toString()) | ||
| const roleTypeProps = document.querySelector('#roletype .role-properties'); | ||
| const statesAndProps = {} | ||
|
|
||
| document.querySelectorAll('#states_and_properties :is(.state, .property)').forEach( | ||
| spSection => { | ||
| const key = spSection.id; | ||
| const value = {}; | ||
| statesAndProps[key] = value; | ||
| value.is = spSection.classList.contains('state') ? 'state' : 'property'; | ||
| value.name = key; | ||
| value.required = false; | ||
| value.disallowed = false; | ||
| value.deprecated = roleTypeProps.querySelector(`[data-deprecated="${key}"]`) ? true : false; | ||
| // TODO: consider adding value information | ||
| } | ||
| ) | ||
|
|
||
| const roleInfo = {}; | ||
|
|
||
|
|
||
| const generateRoleInfoEntry = (roleSection) => { | ||
| const key = roleSection.id; | ||
| const value = {}; | ||
| roleInfo[key] = value; | ||
| value.name = key; | ||
| value.fragID = key; //TODO: [minor] is this duplication really worth it? Role names should be valid IDREFS, no? | ||
| value.parentRoles = []; | ||
| roleSection.querySelectorAll('.role-parent .role-reference').forEach(node => value.parentRoles.push(node.textContent)); | ||
| value.localprops = []; | ||
|
|
||
| const { document } = parseHTML(fs.readFileSync("../aria/index.html").toString()); | ||
| roleSection.querySelectorAll(':is(.role-required-properties, .role-properties, .role-disallowed) :is(.property-reference, .state-reference)').forEach( | ||
| link => { | ||
| const name = link.textContent.split(' ')[0]; //TODO: hack because roletype has (state) inside link (as the only role to have that), cf. TODO: in ariaPreprocessing.js | ||
| const prop = structuredClone(statesAndProps[name]); | ||
| if (key !== 'roletype') prop.deprecated = false; // TODO: should roletype have deprecated=true when the spec lists everything as supported? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain this comment/situation to me... probably a silly question but I'm having trouble wrapping around why is deprecated always set to false, when there are entries in roleInfo.js where deprecated is true?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If an attribute shows up in a characteristics table, then it's not deprecated on this role. (For
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I should add: I'm not saying this is correct (looking again now, I think it's not but I haven't had enough coffee yet). I'm just trying to reproduce roleInfo.js as much as possible right now, with code that's hopefully easier to reason about. (And if we have identified a bug here, then I'll take that as a win π₯³ ) |
||
| if (link.closest('.role-disallowed')) prop["disallowed"] = true; | ||
| if (link.closest('.role-required-properties')) prop["required"] = true; | ||
| value.localprops.push(prop); | ||
| } | ||
| ) | ||
| // TODO: why localprops separately? (localprops are duplicated in allprops; maybe property) | ||
|
|
||
| // mock functions for aria.js | ||
| let updateReferences = () => {}; | ||
| document.URL = ""; | ||
| value.localprops.sort((a, b) => a.name < b.name); | ||
|
|
||
| const script = fs.readFileSync("./common/script/aria.js").toString(); | ||
| const prescript = fs.readFileSync("./common/script/ariaPreprocessing.js").toString(); | ||
| value.allprops = structuredClone(value.localprops); //TODO: why do we duplicate them? Does ariaChild.js need this duplication? (I understand its "allprops" but just "inherited" seems cleaner.) | ||
| roleSection.querySelectorAll('.role-inherited :is(.property-reference, .state-reference)').forEach( | ||
| link => { | ||
| const name = link.textContent.split(' ')[0]; //TODO: hack because roletype has (state) inside link (as the only role to have that), cf. TODO: in ariaPreprocessing.js | ||
| const prop = structuredClone(statesAndProps[name]); | ||
| if (link.closest('.role-disallowed')) prop["disallowed"] = true; | ||
| value.allprops.push(prop); | ||
| } | ||
| ) | ||
| // TODO: sort value.allprops | ||
| } | ||
|
|
||
| // HACK call ariaPreprocessing(), ariaAttributeReferences(), and log out roleInfo with prefix | ||
| const scriptAddition = 'ariaPreprocessing();ariaAttributeReferences();console.log("/* This file is generated - do not modify */ var roleInfo = "+JSON.stringify(roleInfo, null, 2));'; | ||
| document.querySelectorAll('#role_definitions .role').forEach(generateRoleInfoEntry); | ||
|
|
||
| // HACK: eval! | ||
| eval(prescript + script + scriptAddition); | ||
| // sort keys | ||
| const sortKeys = o => Object.keys(o).sort().reduce((r, k) => (r[k] = o[k], r), {}); | ||
| fs.writeFileSync(outputFilename, "/* This file is generated - do not modify */ var roleInfo = " + JSON.stringify(sortKeys(roleInfo), null, 2)); | ||
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 don't know how to test this without merging. Also, this may help with or may be blocked by #2768.
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.
Only way would be through setting up a fork
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 thought this is the cleanest way: when a version is published to github, we update roleinfo.