From 2ecf77a57f64d3238f1ee0576b15e214420aa2f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20C=C3=A1ceres?= Date: Thu, 9 Dec 2021 11:17:41 +1100 Subject: [PATCH 1/6] fix(core/pubsubhub): account for all message types --- src/core/dfn-index.js | 2 ++ src/core/examples.js | 3 --- src/core/issues-notes.js | 3 --- src/core/pubsubhub.js | 10 +++++--- src/type-helper.d.ts | 15 ++++++++++-- tests/spec/core/validators-spec.js | 36 +++++++++++++++++++++++++++++ tests/unit/SpecHelper.js | 5 ++++ tests/unit/core/show-people-spec.js | 22 ++++++++++++++++++ 8 files changed, 85 insertions(+), 11 deletions(-) diff --git a/src/core/dfn-index.js b/src/core/dfn-index.js index b2afe881e1..d6b43b0cc9 100644 --- a/src/core/dfn-index.js +++ b/src/core/dfn-index.js @@ -49,6 +49,8 @@ const CODE_TYPES = new Set([ export function run() { const index = document.querySelector("section#index"); if (!index) { + // See below... + sub("toc", () => {}, { once: true }); return; } diff --git a/src/core/examples.js b/src/core/examples.js index 96dd4124d1..acded3e5de 100644 --- a/src/core/examples.js +++ b/src/core/examples.js @@ -9,7 +9,6 @@ import { addId, getIntlData } from "./utils.js"; import css from "../styles/examples.css.js"; import { html } from "./import-maps.js"; -import { pub } from "./pubsubhub.js"; export const name = "core/examples"; @@ -93,7 +92,6 @@ export function run() { const id = addId(example, "example", title || String(number)); const selfLink = div.querySelector("a.self-link"); selfLink.href = `#${id}`; - pub("example", report); } else { const inAside = !!example.closest("aside"); if (!inAside) ++number; @@ -113,7 +111,6 @@ export function run() { const selfLink = div.querySelector("a.self-link"); selfLink.href = `#${div.id}`; example.replaceWith(div); - if (!inAside) pub("example", report); } }); } diff --git a/src/core/issues-notes.js b/src/core/issues-notes.js index 4476164e52..4fa8fc9dea 100644 --- a/src/core/issues-notes.js +++ b/src/core/issues-notes.js @@ -20,8 +20,6 @@ import { } from "./utils.js"; import css from "../styles/issues-notes.css.js"; import { html } from "./import-maps.js"; -import { pub } from "./pubsubhub.js"; - export const name = "core/issues-notes"; const localizationStrings = { @@ -186,7 +184,6 @@ function handleIssues(ins, ghIssues, conf) { const level = parents(titleParent, "section").length + 2; titleParent.setAttribute("aria-level", level); } - pub(report.type, report); }); makeIssueSectionSummary(issueList); } diff --git a/src/core/pubsubhub.js b/src/core/pubsubhub.js index 15d158fe12..52a1249656 100644 --- a/src/core/pubsubhub.js +++ b/src/core/pubsubhub.js @@ -12,9 +12,14 @@ export const name = "core/pubsubhub"; const subscriptions = new Map(); +/** + * + * @param {EventTopic} topic + * @param {...any} data + */ export function pub(topic, ...data) { if (!subscriptions.has(topic)) { - return; // Nothing to do... + throw new Error(`No subscribers for topic "${topic}".`); } Array.from(subscriptions.get(topic)).forEach(cb => { try { @@ -37,8 +42,7 @@ export function pub(topic, ...data) { } /** * Subscribes to a message type. - * - * @param {string} topic The topic to subscribe to (e.g., "start-all") + * @param {EventTopic} topic The topic to subscribe to * @param {Function} cb Callback function * @param {Object} [opts] * @param {Boolean} [opts.once] Add prop "once" for single notification. diff --git a/src/type-helper.d.ts b/src/type-helper.d.ts index bc61dd7d94..72a2ff4be1 100644 --- a/src/type-helper.d.ts +++ b/src/type-helper.d.ts @@ -134,7 +134,7 @@ type LicenseInfo = { * The short linking text of license. */ short: string; -} +}; type ResourceHintOption = { /** @@ -223,6 +223,16 @@ type PersonExtras = { href?: string; }; +type EventTopic = + | "amend-user-config" + | "beforesave" + | "end-all" + | "error" + | "plugins-done" + | "start-all" + | "toc" + | "warn"; + type DefinitionValidator = ( /** Text to validate. */ text: string, @@ -231,4 +241,5 @@ type DefinitionValidator = ( /** The element from which the validation originated. */ element: HTMLElement, /** The name of the plugin originating the validation. */ - pluginName: string) => boolean; + pluginName: string +) => boolean; diff --git a/tests/spec/core/validators-spec.js b/tests/spec/core/validators-spec.js index 27c369e740..6a86ba1f97 100644 --- a/tests/spec/core/validators-spec.js +++ b/tests/spec/core/validators-spec.js @@ -4,6 +4,7 @@ import { validateMimeType, validateQuotedString, } from "../../../src/core/dfn-validators.js"; +import { sub } from "../../../src/core/pubsubhub.js"; describe("Core - Validators", () => { describe("validateDOMName", () => { @@ -30,6 +31,13 @@ describe("Core - Validators", () => { it("generates an error if the element name is not valid", () => { const elements = ["my element", "crypto$", "🪳", "-something", ""]; for (const element of elements) { + sub( + "error", + () => { + expect(true).toBeTrue(); + }, + { once: true } + ); const dfn = document.createElement("dfn"); const context = `element name: ${element}`; expect(validateDOMName(element, "element", dfn, "foo/bar")) @@ -58,6 +66,13 @@ describe("Core - Validators", () => { it("generates an error if the attribute name is invalid", () => { const attributes = ["-crossorigin", "-whatever-", "aria-😇"]; for (const attribute of attributes) { + sub( + "error", + () => { + expect(true).toBeTrue(); + }, + { once: true } + ); const context = `attribute name: ${attribute}`; const dfn = document.createElement("dfn"); expect(validateDOMName(attribute, "attribute", dfn, "foo/bar")) @@ -79,6 +94,13 @@ describe("Core - Validators", () => { "text/plain", ]; for (const mimeType of mimeTypes) { + sub( + "error", + () => { + expect(true).toBeTrue(); + }, + { once: true } + ); const dfn = document.createElement("dfn"); const context = `mimeType: ${mimeType}`; expect(validateMimeType(mimeType, "mimetype", dfn, "foo/bar")) @@ -149,6 +171,13 @@ describe("Core - Validators", () => { it("generates no error if the name is valid", () => { const names = ["foo", "bar", "baz", "quux"]; for (const name of names) { + sub( + "error", + () => { + expect(true).toBeTrue(); + }, + { once: true } + ); const dfn = document.createElement("dfn"); const context = `name: ${name}`; expect(validateCommonName(name, "event", dfn, "foo/bar")) @@ -163,6 +192,13 @@ describe("Core - Validators", () => { it("it generates an error if the name is not valid", () => { const names = ["my event", "crypto$", "🪳", "-something"]; for (const name of names) { + sub( + "error", + () => { + expect(true).toBeTrue(); + }, + { once: true } + ); const dfn = document.createElement("dfn"); const context = `invalid name: ${name}`; expect(validateCommonName(name, "event", dfn, "foo/bar")) diff --git a/tests/unit/SpecHelper.js b/tests/unit/SpecHelper.js index 386f467464..91deb53911 100644 --- a/tests/unit/SpecHelper.js +++ b/tests/unit/SpecHelper.js @@ -14,6 +14,11 @@ export function makePluginDoc( plugins, { config = {}, head = ``, body = "" } = {} ) { + plugins = [ + `/src/core/ui.js`, // Needed for "start-all" event + `/src/core/dfn.js`, // Needed for "plugins-done" event, + ...plugins, + ]; return getDoc(` diff --git a/tests/unit/core/show-people-spec.js b/tests/unit/core/show-people-spec.js index da24c9bb9b..8180428106 100644 --- a/tests/unit/core/show-people-spec.js +++ b/tests/unit/core/show-people-spec.js @@ -1,5 +1,6 @@ import { html } from "../../../src/core/import-maps.js"; import showPeople from "../../../src/core/templates/show-people.js"; +import { sub } from "../../../src/core/pubsubhub.js"; describe("Core - Templates - Show People", () => { describe("showPeople", () => { @@ -96,6 +97,13 @@ describe("Core - Templates - Show People", () => { }); it("identifies valid and invalid ORCIDs", async () => { + sub( + "error", + () => { + expect(true).toBeTrue(); + }, + { once: true } + ); const people = [ { name: "Valid 1", @@ -136,6 +144,13 @@ describe("Core - Templates - Show People", () => { }); it("ignores companyURL when company is missing", () => { + sub( + "warn", + () => { + expect(true).toBeTrue(); + }, + { once: true } + ); const render = html.bind(document.createDocumentFragment()); const person = { name: "name", @@ -177,6 +192,13 @@ describe("Core - Templates - Show People", () => { }); it("filters out editors with invalid extras", () => { + sub( + "error", + () => { + expect(true).toBeTrue(); + }, + { once: true } + ); const render = html.bind(document.createDocumentFragment()); const people = [ { name: "wrong type", extras: "" }, From 377b848f72f93180e86ff7a6d97738751f01eff7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20C=C3=A1ceres?= Date: Thu, 9 Dec 2021 19:57:26 +1100 Subject: [PATCH 2/6] Fix failing test --- tests/unit/core/show-people-spec.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/core/show-people-spec.js b/tests/unit/core/show-people-spec.js index 8180428106..39963ba647 100644 --- a/tests/unit/core/show-people-spec.js +++ b/tests/unit/core/show-people-spec.js @@ -5,6 +5,13 @@ import { sub } from "../../../src/core/pubsubhub.js"; describe("Core - Templates - Show People", () => { describe("showPeople", () => { it("handles empty person", () => { + sub( + "error", + () => { + expect(true).toBeTrue(); + }, + { once: true } + ); const render = html.bind(document.createDocumentFragment()); const person = {}; const result = render`${showPeople({ people: [person] }, "people")}`; From 16e5c6e760049f8af5b89abf78b6fb909debbc17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20C=C3=A1ceres?= Date: Thu, 9 Dec 2021 20:01:42 +1100 Subject: [PATCH 3/6] don't actaully test --- tests/unit/core/show-people-spec.js | 32 ++++------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/tests/unit/core/show-people-spec.js b/tests/unit/core/show-people-spec.js index 39963ba647..964bd78682 100644 --- a/tests/unit/core/show-people-spec.js +++ b/tests/unit/core/show-people-spec.js @@ -5,13 +5,7 @@ import { sub } from "../../../src/core/pubsubhub.js"; describe("Core - Templates - Show People", () => { describe("showPeople", () => { it("handles empty person", () => { - sub( - "error", - () => { - expect(true).toBeTrue(); - }, - { once: true } - ); + sub("error", () => {}, { once: true }); const render = html.bind(document.createDocumentFragment()); const person = {}; const result = render`${showPeople({ people: [person] }, "people")}`; @@ -104,13 +98,7 @@ describe("Core - Templates - Show People", () => { }); it("identifies valid and invalid ORCIDs", async () => { - sub( - "error", - () => { - expect(true).toBeTrue(); - }, - { once: true } - ); + sub("error", () => {}, { once: true }); const people = [ { name: "Valid 1", @@ -151,13 +139,7 @@ describe("Core - Templates - Show People", () => { }); it("ignores companyURL when company is missing", () => { - sub( - "warn", - () => { - expect(true).toBeTrue(); - }, - { once: true } - ); + sub("warn", () => {}, { once: true }); const render = html.bind(document.createDocumentFragment()); const person = { name: "name", @@ -199,13 +181,7 @@ describe("Core - Templates - Show People", () => { }); it("filters out editors with invalid extras", () => { - sub( - "error", - () => { - expect(true).toBeTrue(); - }, - { once: true } - ); + sub("error", () => {}, { once: true }); const render = html.bind(document.createDocumentFragment()); const people = [ { name: "wrong type", extras: "" }, From 090fd2085c2dfcd488253096e453e8e776b0141c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20C=C3=A1ceres?= Date: Thu, 9 Dec 2021 20:03:51 +1100 Subject: [PATCH 4/6] don't actaully test --- tests/spec/core/validators-spec.js | 40 ++++-------------------------- 1 file changed, 5 insertions(+), 35 deletions(-) diff --git a/tests/spec/core/validators-spec.js b/tests/spec/core/validators-spec.js index 6a86ba1f97..f26306e093 100644 --- a/tests/spec/core/validators-spec.js +++ b/tests/spec/core/validators-spec.js @@ -31,13 +31,7 @@ describe("Core - Validators", () => { it("generates an error if the element name is not valid", () => { const elements = ["my element", "crypto$", "🪳", "-something", ""]; for (const element of elements) { - sub( - "error", - () => { - expect(true).toBeTrue(); - }, - { once: true } - ); + sub("error", () => {}, { once: true }); const dfn = document.createElement("dfn"); const context = `element name: ${element}`; expect(validateDOMName(element, "element", dfn, "foo/bar")) @@ -66,13 +60,7 @@ describe("Core - Validators", () => { it("generates an error if the attribute name is invalid", () => { const attributes = ["-crossorigin", "-whatever-", "aria-😇"]; for (const attribute of attributes) { - sub( - "error", - () => { - expect(true).toBeTrue(); - }, - { once: true } - ); + sub("error", () => {}, { once: true }); const context = `attribute name: ${attribute}`; const dfn = document.createElement("dfn"); expect(validateDOMName(attribute, "attribute", dfn, "foo/bar")) @@ -94,13 +82,7 @@ describe("Core - Validators", () => { "text/plain", ]; for (const mimeType of mimeTypes) { - sub( - "error", - () => { - expect(true).toBeTrue(); - }, - { once: true } - ); + sub("error", () => {}, { once: true }); const dfn = document.createElement("dfn"); const context = `mimeType: ${mimeType}`; expect(validateMimeType(mimeType, "mimetype", dfn, "foo/bar")) @@ -171,13 +153,7 @@ describe("Core - Validators", () => { it("generates no error if the name is valid", () => { const names = ["foo", "bar", "baz", "quux"]; for (const name of names) { - sub( - "error", - () => { - expect(true).toBeTrue(); - }, - { once: true } - ); + sub("error", () => {}, { once: true }); const dfn = document.createElement("dfn"); const context = `name: ${name}`; expect(validateCommonName(name, "event", dfn, "foo/bar")) @@ -192,13 +168,7 @@ describe("Core - Validators", () => { it("it generates an error if the name is not valid", () => { const names = ["my event", "crypto$", "🪳", "-something"]; for (const name of names) { - sub( - "error", - () => { - expect(true).toBeTrue(); - }, - { once: true } - ); + sub("error", () => {}, { once: true }); const dfn = document.createElement("dfn"); const context = `invalid name: ${name}`; expect(validateCommonName(name, "event", dfn, "foo/bar")) From eccd45a10d4bf4a1cb2914f85c889d0a4158da3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20C=C3=A1ceres?= Date: Thu, 9 Dec 2021 20:26:45 +1100 Subject: [PATCH 5/6] don't actaully test --- tests/spec/core/validators-spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/spec/core/validators-spec.js b/tests/spec/core/validators-spec.js index f26306e093..263c0aa570 100644 --- a/tests/spec/core/validators-spec.js +++ b/tests/spec/core/validators-spec.js @@ -137,6 +137,7 @@ describe("Core - Validators", () => { "-something", ]; for (const name of names) { + sub("error", () => {}, { once: true }); const dfn = document.createElement("dfn"); const context = `invalid name: ${name}`; expect(validateQuotedString(name, "permission", dfn, "foo/bar")) From 9e02a6d7a77438cce1bd6778d573363553d2dfa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20C=C3=A1ceres?= Date: Sat, 18 Dec 2021 13:40:18 +1100 Subject: [PATCH 6/6] Review feedback --- tests/unit/SpecHelper.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/unit/SpecHelper.js b/tests/unit/SpecHelper.js index 91deb53911..6ed547ef51 100644 --- a/tests/unit/SpecHelper.js +++ b/tests/unit/SpecHelper.js @@ -15,8 +15,9 @@ export function makePluginDoc( { config = {}, head = ``, body = "" } = {} ) { plugins = [ - `/src/core/ui.js`, // Needed for "start-all" event - `/src/core/dfn.js`, // Needed for "plugins-done" event, + "/src/core/base-runner.js", + "/src/core/ui.js", // Needed for "start-all" event + "/src/core/dfn.js", // Needed for "plugins-done" event, ...plugins, ]; return getDoc(` @@ -29,9 +30,7 @@ export function makePluginDoc(