[CLOSED] Event types dispatched from NodeConnection are incorrect

[core-ai-bot]

Issue by jasonsanjose
Thursday Feb 20, 2014 at 17:55 GMT
Originally opened as adobe/brackets#6933

---

1. Open dev tools
2. Install this extension to demonstrate the bug https://github.com/jasonsanjose/brackets-simple-node
3. Observe console below that outputs the jQuery event when listening for a base.log event type. The actual event type is base where we expected base.log:

> [node-log 9:16:55 AM] hello from SimpleDomain NodeDebugUtils.js:117
> x.Event
currentTarget: NodeConnection
data: undefined
delegateTarget: NodeConnection
handleObj: Object
isTrigger: 2
jQuery20106182983780745417: true
namespace: "log"
namespace_re: /(^|\.)log(\.|$)/
result: undefined
target: NodeConnection
timeStamp: 1392916615716
type: "base"
__proto__: Object
> [brackets-simple-node] Memory: 96583680 of 4294967296 bytes free (2%)

Explanation:

Following the event handling convention here: https://github.com/adobe/brackets/wiki/Brackets-Node-Process:-Overview-for-Developers#wiki-architecture, the demo extension adds an event listener for log in the base domain, e.g. $(connection).on("base.log", myEventHandler). See https://github.com/jasonsanjose/brackets-simple-node/blob/master/main.js#L65.

When the event is fired, the debug statement in the console above shows the event type type: "base" which is incorrect.

My memory is a bit hazy on this, and I thought I asked@joelrbrandt about it, but I always thought that this notation would screw up somehow with jQuery's namespace notation. It never really manifested as a bug for a few reasons that I can tell:

1. Most of our node domains only ever register 1 type of event. So there wasn't an opportunity to mishandle anything yet.
   2.@iwehrman introduced NodeDomain which may mask the problem by adding it's own namespace when listening to events from NodeConnection before re-dispatching them. e.g. NodeDomain listens for the change event fileWatcher.change.NodeDomainEvent then dispatches simply change. However, the event type that comes back from NodeConnection is still fileWatcher with a namespace of change.

The relevant jQuery source is here https://github.com/jquery/jquery/blob/master/src/event.js#L238.

I ran into an issue when working on a node extension that for some reason never completed loading. What I found was that NodeConnection itself listens for base.newDomains before resolving the promise for NodeConnection.loadDomains(). When stepping through the code, I saw the base.newDomains event fire, but it was never handled by the event listener.

[core-ai-bot]

Comment by jasonsanjose
Thursday Feb 20, 2014 at 18:00 GMT

---

It's worth noting that in the unit tests we don't actually verify the event type, only the payload: https://github.com/adobe/brackets/blob/master/test/spec/NodeConnection-test.js#L194

[core-ai-bot]

Comment by jasonsanjose
Monday Feb 24, 2014 at 17:21 GMT

---

Fixed in #6942. Closing.