Cometd 2.4.0 Javascript AMD compliant patch

34 views
Skip to first unread message

sarurobi

unread,
Feb 17, 2012, 5:36:05 AM2/17/12
to cometd-users
Hi Simone,
don't know if I made something wrong, but I had to patch the latest js
to make it AMD compliant and let it works with the new dojo micro
kernel.
With that I was able to write my code in this way :

require(["dojox/cometd"],function(cometd){


// Do something with cometd :

cometd.addListener("/meta/connect",_onConnect);

.... ecc. ...

});


Here's my (really dirty) patch, if it's useful:


---
WebContent/dojox/cometd.js | 9 ++++++++-
WebContent/org/cometd.js | 11 ++++++++++-
3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/WebContent/dojox/cometd.js b/WebContent/dojox/cometd.js
index db5fab0..b0632eb 100644
--- a/WebContent/dojox/cometd.js
+++ b/WebContent/dojox/cometd.js
@@ -14,12 +14,15 @@
* limitations under the License.
*/

+define("dojox/cometd",["org/cometd","dojo/io/
script"],function(_cometd,script) {

-
+/*
dojo.provide('dojox.cometd');
dojo.registerModulePath('org','../org');
dojo.require('org.cometd');
dojo.require('dojo.io.script');
+*/
+

// Remap cometd JSON functions to dojo JSON functions
org.cometd.JSON.toJSON = dojo.toJson;
@@ -128,3 +131,7 @@ dojox.cometd._metaConnectEvent = function(event)

dojox.cometd.addListener('/meta/handshake', dojox.cometd,
dojox.cometd._metaHandshakeEvent);
dojox.cometd.addListener('/meta/connect', dojox.cometd,
dojox.cometd._metaConnectEvent);
+
+return dojox.cometd;
+
+});
diff --git a/WebContent/org/cometd.js b/WebContent/org/cometd.js
index 01bea94..c17955b 100644
--- a/WebContent/org/cometd.js
+++ b/WebContent/org/cometd.js
@@ -14,7 +14,9 @@
* limitations under the License.
*/

-if (typeof dojo !== 'undefined')
+define("org/cometd",[],function() {
+
+ if (typeof dojo !== 'undefined')
{
dojo.provide('org.cometd');
}
@@ -2953,3 +2955,10 @@ org.cometd.Cometd = function(name)
}
};

+return org.cometd;
+
+
+});
+
+
+
--
1.7.5.4

Simone Bordet

unread,
Feb 22, 2012, 4:12:17 AM2/22/12
to cometd...@googlegroups.com
Hi,

On Fri, Feb 17, 2012 at 11:36, sarurobi <vittorio....@gmail.com> wrote:
> Hi Simone,
> don't know if I made something wrong, but I had to patch the latest js
> to make it AMD compliant and let it works with the new dojo micro
> kernel.

Thanks, we'll work towards this integration as soon as possible.

Simon
--
http://cometd.org
http://intalio.com
http://bordet.blogspot.com
----
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz

neek

unread,
Feb 27, 2012, 10:29:48 PM2/27/12
to cometd-users
Guys,

Is there any official release for this coming soon?

If not, sarurobi, thank you very much for the patch, could you please
release a clean file? I've had trouble taking your paste from this
web page and using it as a patch file due to line breaks and so on.

The closest I got still has trouble on line 23:

[neek dojo (develop)]$ patch -p2 < cometd_amd.patch
patching file dojox/cometd.js
patch: **** malformed patch at line 24: @@ -128,3 +131,7 @@
dojox.cometd._metaConnectEvent = function(event)

I'm not a patch guru and don't want to waste time massaging a broken
file.

I believe you can attach files to this group, though not to specific
posts. Alternatively, raise an issue on http://bugs.cometd.org/ and
attach it there, or mail it to me or Simone and we can probably sort
it out.

Cometd not supporting AMD is going to be the one thing preventing many
projects from switching to full async:true behaviour for the Dojo
loader. As I understand it from e.g.
http://dojotoolkit.org/reference-guide/dojo/domReady.html#sync-loader
any dojo.require() calls will trip up the new AMD plugins like dojo/
domReady!.

neek

unread,
Feb 27, 2012, 10:33:43 PM2/27/12
to cometd-users
I just noticed there's already a JIRA issue for this:

"Update Dojo client to use AMD"
http://bugs.cometd.org/browse/COMETD-336

That would seem a good place to attach this patch.

Vittorio Ballestra

unread,
Feb 28, 2012, 2:02:26 AM2/28/12
to cometd...@googlegroups.com

Ok. I Will append the patch in a few hours. Keep in mind it's only a dirty hack that 'just works' but breaks jquery compatibility :-)

--
You received this message because you are subscribed to the Google Groups "cometd-users" group.
To post to this group, send email to cometd...@googlegroups.com
To unsubscribe from this group, send email to cometd-users...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/cometd-users

Visit the CometD website at http://www.cometd.org

sarurobi

unread,
Feb 28, 2012, 4:44:03 AM2/28/12
to cometd-users
Sorry, that's the best I could do as I don't have a Jira account
neither I was able to "attach" something (BTW: how the hell it is
supposed to work with google groups ?? )

Here's a link to my dropbox public folder: http://dl.dropbox.com/u/11489985/p

apply the patch with :

patch -p2 -R <p

In the folder where you have the dojox folder and cometd.js and
everything should be fine (I've done a little test and it worked).

neek

unread,
Mar 20, 2012, 3:22:27 AM3/20/12
to cometd-users


On Feb 28, 4:44 pm, sarurobi <vittorio.balles...@gmail.com> wrote:
> Sorry, that's the best I could do as I don't have a Jira account
> neither I was able to "attach" something (BTW: how the hell it is
> supposed to work with google groups ?? )

Yeah, last time I looked you could attach a file to a pool of files on
the group itself, but not to individual posts or 'Discussions'. Seems
crazy.

> Here's a link to my dropbox public folder:http://dl.dropbox.com/u/11489985/p

Thank you very much. Your patch seems to be backwards but I can see
what you were doing. I've patched up cometd.js files, and also
reload.js and ReloadExtension.js since I use that. Other extensions
would need patching accordingly.

However, my ready() events then fail to work, which seems to be a dojo
loader bug.

I'm not sure your code is correct. Where you have e.g.:

define("org/cometd",[],function() {

at the top of org/cometd.js, I'm used to syntax like this:

define( [ "dojo/_base/declare", .. list of dependencies .. ],
function(declare, .. other deps ..) {
return declare("org.cometd", null, { .. body here .. })
})

Something alone those lines, anyway. I've tried updating the code to
match this syntax, but I got lost converting the cometd objects to a
format that's compatible with "return declare", and I have a feeling
this is a wasted effort anyway since Simone said he was working toward
an official AMD patch (though I've seen no word for almost a month on
that).

Due to the loader bug and ready() event not working, this simple AMD
conversion is a non-starter for me. I'm going to have to leave
async:false in the data-dojo-config, and hope the cometd project comes
through with an AMD compliant version before too long.

Nick
Reply all
Reply to author
Forward
0 new messages