From d2897f0a2fbdb45458c998296c5e6ed94f9150c0 Mon Sep 17 00:00:00 2001 From: Vladimir Rangelov Date: Tue, 18 Nov 2025 17:11:41 +0000 Subject: [PATCH 1/2] feat: Bidirectional events should come in as 'on' --- .../javascript/src/shared/Events/index.mjs | 5 ++--- src/macrofier/engine.mjs | 18 ------------------ src/shared/modules.mjs | 10 ++++------ test_sdk/openrpc/advanced.json | 10 ++++------ 4 files changed, 10 insertions(+), 33 deletions(-) diff --git a/languages/javascript/src/shared/Events/index.mjs b/languages/javascript/src/shared/Events/index.mjs index 96bae3dd..6aac2ab9 100644 --- a/languages/javascript/src/shared/Events/index.mjs +++ b/languages/javascript/src/shared/Events/index.mjs @@ -145,8 +145,7 @@ const doListen = function(module, event, callback, context, once, internal=false if (Object.values(listeners.get(key)).length === 0) { const args = Object.assign({ listen: true }, context) - // TODO: Is subscriber -> notifer required to be a simple transform (drop 'on'?) - const subscriber = module + '.on' + event[0].toUpperCase() + event.substring(1) + const subscriber = module + '.' + event const notifier = module + '.' + event Gateway.subscribe(notifier, (params) => { @@ -252,7 +251,7 @@ export const prioritize = function(...args) { const unsubscribe = (key, context) => { const [module, event] = key.split('.').slice(0, 2) const args = Object.assign({ listen: false }, context) - Gateway.request(module + '.on' + event[0].toUpperCase() + event.substr(1), args) + Gateway.request(module + '.' + event, args) Gateway.unsubscribe(`${module}.${event}`) } diff --git a/src/macrofier/engine.mjs b/src/macrofier/engine.mjs index 07d2432c..063dcf95 100644 --- a/src/macrofier/engine.mjs +++ b/src/macrofier/engine.mjs @@ -346,15 +346,6 @@ const rpcMethodsOrEmptyArray = compose( // Pick events out of the methods array const eventsOrEmptyArray = compose( option([]), - map(filter(validEvent)), - // Maintain the side effect of process.exit here if someone is violating the rules - map(map(e => { - if (!e.name.match(/on[A-Z]/)) { - console.error(`ERROR: ${e.name} method is tagged as an event, but does not match the pattern "on[A-Z]"`) - process.kill(process.pid) // Using process.kill so that other worspaces all exit (and don't bury this error w/ logs) - } - return e - })), map(filter(isPublicEventMethod)), getMethods ) @@ -390,15 +381,6 @@ const providedCapabilitiesOrEmptyArray = compose( // Pick providers out of the methods array const providersOrEmptyArray = compose( option([]), - map(filter(validEvent)), - // Maintain the side effect of process.exit here if someone is violating the rules - map(map(e => { - if (!e.name.match(/on[A-Z]/)) { - console.error(`ERROR: ${e.name} method is tagged as a provider, but does not match the pattern "on[A-Z]"`) - process.exit(1) // Non-zero exit since we don't want to continue. Useful for CI/CD pipelines. - } - return e - })), map(filter(isProviderInterfaceMethod)), getMethods ) diff --git a/src/shared/modules.mjs b/src/shared/modules.mjs index 960208cf..41b1204e 100644 --- a/src/shared/modules.mjs +++ b/src/shared/modules.mjs @@ -432,12 +432,12 @@ const eventDefaults = event => { const createNotifierFromProperty = (property, type='Changed') => { const notifier = JSON.parse(JSON.stringify(property)) - notifier.name = methodRename(notifier, name => name + type) + notifier.name = methodRename(notifier, name => 'on' + name.charAt(0).toUpperCase() + name.substring(1) + type) Object.assign(notifier.tags.find(t => t.name.startsWith('property')), { name: 'notifier', 'x-notifier-for': property.name, - 'x-event': methodRename(notifier, name => 'on' + name.charAt(0).toUpperCase() + name.substring(1)) + 'x-event': notifier.name }) notifier.params.push(notifier.result) @@ -1110,13 +1110,12 @@ const generateEventSubscribers = json => { const tag = notifier.tags.find(tag => tag.name === 'notifier') // if there's an x-event extension, this denotes an editorially created subscriber if (!tag['x-event']) { - tag['x-event'] = methodRename(notifier, name => 'on' + name.charAt(0).toUpperCase() + name.substring(1)) + tag['x-event'] = notifier.name } - const subscriber = json.methods.find(method => method.name === tag['x-event']) + const subscriber = json.methods.find(method => method.tags.find(t => t['x-notifier'] === notifier.name)) if (!subscriber) { const subscriber = JSON.parse(JSON.stringify(notifier)) - subscriber.name = methodRename(subscriber, name => 'on' + name.charAt(0).toUpperCase() + name.substring(1)) subscriber.params.pop() subscriber.params.push({ name: 'listen', @@ -2085,7 +2084,6 @@ export { getEnums, getTypes, getEvents, - getPublicEvents, getSchemas, getParamsFromMethod, fireboltize, diff --git a/test_sdk/openrpc/advanced.json b/test_sdk/openrpc/advanced.json index 4d748b9b..a04b3e40 100644 --- a/test_sdk/openrpc/advanced.json +++ b/test_sdk/openrpc/advanced.json @@ -10,8 +10,7 @@ "name": "plainEvent", "tags": [ { - "name": "notifier", - "x-event": "Advanced.onPlainEvent" + "name": "notifier" }, { "name": "capabilities", @@ -43,7 +42,7 @@ ] }, { - "name": "eventWithContext", + "name": "onEventWithContext", "tags": [ { "name": "notifier", @@ -96,11 +95,10 @@ ] }, { - "name": "eventWithTwoContext", + "name": "onEventWithTwoContext", "tags": [ { - "name": "notifier", - "x-event": "Advanced.onEventWithTwoContext" + "name": "notifier" }, { "name": "capabilities", From cd89242375e7eda25cb8cd8f7180d9bb49c3cca0 Mon Sep 17 00:00:00 2001 From: Vladimir Rangelov Date: Wed, 19 Nov 2025 14:19:30 +0000 Subject: [PATCH 2/2] fix: Fix unit tests --- languages/javascript/src/shared/Prop/index.mjs | 3 ++- src/macrofier/engine.mjs | 4 ++-- test_sdk/suite/properties-context.test.js | 4 ++-- test_sdk/suite/properties.test.js | 10 +++++----- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/languages/javascript/src/shared/Prop/index.mjs b/languages/javascript/src/shared/Prop/index.mjs index 33aba7f8..5d5ee55e 100644 --- a/languages/javascript/src/shared/Prop/index.mjs +++ b/languages/javascript/src/shared/Prop/index.mjs @@ -21,7 +21,8 @@ function prop( if (immutable) { throw new Error('Cannot subscribe to an immutable property'); } - return Events.listen(moduleName, key + 'Changed', ...Object.values(params), callbackOrValue); + const subscriber = 'on' + key[0].toUpperCase() + key.substring(1) + 'Changed' + return Events.listen(moduleName, subscriber, ...Object.values(params), callbackOrValue); } else if (type === 'setter') { // setter if (immutable) { diff --git a/src/macrofier/engine.mjs b/src/macrofier/engine.mjs index 063dcf95..8c38c567 100644 --- a/src/macrofier/engine.mjs +++ b/src/macrofier/engine.mjs @@ -402,7 +402,7 @@ const getModuleName = json => { return json ? (json.title || (json.info ? json.info.title : 'Unknown')) : 'Unknown' } -const makeEventName = x => methodName(x)[2].toLowerCase() + methodName(x).substr(3) // onFooBar becomes fooBar +const makeEventName = x => methodName(x) // onFooBar remains onFooBar const makeProviderMethod = x => x.name["onRequest".length].toLowerCase() + x.name.substr("onRequest".length + 1) // onRequestChallenge becomes challenge const generateAggregateMacros = (platformApi, appApi, additional, templates, library) => { @@ -1767,7 +1767,7 @@ function insertMethodMacros(template, methodObj, platformApi, appApi, templates, .replace(/\$\{method\.context\.count}/g, method.context ? method.context.length : 0) .replace(/\$\{method\.deprecation\}/g, deprecation) .replace(/\$\{method\.Name\}/g, method.name[0].toUpperCase() + method.name.substr(1)) - .replace(/\$\{event\.name\}/g, method.name.toLowerCase()[2] + method.name.substr(3)) + .replace(/\$\{event\.name\}/g, method.name) .replace(/\$\{event\.params\}/g, eventParams) .replace(/\$\{event\.params\.table\.rows\}/g, eventParamsRows) .replace(/\$\{if\.event\.params\}(.*?)\$\{end\.if\.event\.params\}/gms, event && event.params.length ? '$1' : '') diff --git a/test_sdk/suite/properties-context.test.js b/test_sdk/suite/properties-context.test.js index c45cb6ea..17583e19 100644 --- a/test_sdk/suite/properties-context.test.js +++ b/test_sdk/suite/properties-context.test.js @@ -114,13 +114,13 @@ test('Context Property set', () => { }); test('Event with single context param', () => { - Advanced.listen("eventWithContext", "some-app", (data) => { + Advanced.listen("onEventWithContext", "some-app", (data) => { expect(contextSentToEvent).toBe(true) }) }) test('Event with two context params', () => { - Advanced.listen("eventWithTwoContext", "some-app", "inactive", (data) => { + Advanced.listen("onEventWithTwoContext", "some-app", "inactive", (data) => { expect(bothContextSentToEvent).toBe(true) }) }) \ No newline at end of file diff --git a/test_sdk/suite/properties.test.js b/test_sdk/suite/properties.test.js index 551bfb6f..b2ba189b 100644 --- a/test_sdk/suite/properties.test.js +++ b/test_sdk/suite/properties.test.js @@ -69,7 +69,7 @@ test('Basic Property subscribe', () => { let p = Simple.plainProperty(value => { expect(value.value).toBe("value 123") }) - MockTransport.event("Simple","plainPropertyChanged", "value 123"); + MockTransport.event("Simple","onPlainPropertyChanged", "value 123"); return p; }); @@ -90,12 +90,12 @@ test('Basic Property set with null', () => { expect(propertySetterWasTriggeredWithValue).toBe(true) }); -//test listen to "plainPropertyChanged" event +//test listen to "onPlainPropertyChanged" event test('Basic Property subscribe to event', () => { - Simple.clear("plainPropertyChanged"); - let p = Simple.listen("plainPropertyChanged", value => { + Simple.clear("onPlainPropertyChanged"); + let p = Simple.listen("onPlainPropertyChanged", value => { expect(value.value).toBe( "value 123") }) - MockTransport.event("Simple","plainPropertyChanged", "value 123"); + MockTransport.event("Simple","onPlainPropertyChanged", "value 123"); return p; }); \ No newline at end of file