Skip to content

fix(ts): use correct typing for JSDoc COMMUNICATION_MESSAGE_TYPE - #6

Merged
rugk merged 3 commits into
masterfrom
fix/jsdoc
Aug 19, 2025
Merged

fix(ts): use correct typing for JSDoc COMMUNICATION_MESSAGE_TYPE#6
rugk merged 3 commits into
masterfrom
fix/jsdoc

Conversation

@rugk

@rugk rugk commented Aug 19, 2025

Copy link
Copy Markdown
Member

fix(ts): use correct typing for JSDoc COMMUNICATION_MESSAGE_TYPE

Fixes #5

@rugk rugk left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AI suggested these additional changes BTW, too, but the were too risky for me to include, through a cautionary look it seems to look good (but for AI stuff that means nothing, it often looks good on the first sight):

diff --git a/BrowserCommunication.js b/BrowserCommunication.js
index e8b0484..8451d1b 100644
--- a/BrowserCommunication.js
+++ b/BrowserCommunication.js
@@ -75,40 +75,25 @@ function handleMessages(request, sender, sendResponse) {
 
     if (!(messageType in callbacks) || callbacks[messageType].length === 0) {
         console.warn(`No callbacks for message type "${messageType}" registered.`);
-        return true;
+        return;
     }
 
     // call all callbacks and keep return values
     const promises = [];
-    let gotTrueAsReturn = false;
     for (const callback of callbacks[messageType]) {
         const returnValue = callback(request, sender, sendResponse);
 
-        // notice if return value is just "true"
-        if (returnValue === true) {
-            gotTrueAsReturn = true;
-            continue;
+        // If the callback returns a Promise, add it to the list
+        if (returnValue && typeof returnValue.then === "function") {
+            promises.push(returnValue);
         }
-
-        // return value should be a Promise
-        promises.push(returnValue);
     }
 
-    // handle returning
-    if (gotTrueAsReturn) {
-        if (callbacks[messageType].length !== 1) {
-            // if it was not the only callback, then show a real error
-            console.error(`At least one callback for message type "${messageType}" returned the legacy value "true".
-            As you have registered ${callbacks[messageType].length} listeners this may lead to errors.`);
-        } else {
-            // show warning as this behaviour is discouraged
-            console.warn(`At least one callback for message type "${messageType}" returned the legacy value "true". Please return a Promise instead.`);
-        }
-
-        return true;
+    if (promises.length > 0) {
+        // If there are async handlers, return a Promise
+        return Promise.all(promises);
     }
-
-    return Promise.all(promises);
+    // Otherwise, return nothing (void)
 }
 
 /**

@rugk
rugk merged commit 7ef173e into master Aug 19, 2025
rugk added a commit that referenced this pull request Aug 19, 2025
…s correctly

AI-generated by Copilot, so I do not fully trust this.

It is based on #6 and it "fixed" this while wanting to fix the return
type typing issue here (f6e03a6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enforce proper JSDoc and fix EsLint issues

1 participant