upgraded to SignalR 2.4.1

gregveres commented 4 years ago

I diff'd the existing signalR.js file against the original v2.2.1 to see what changes this package made to the file. I then brought over the new 2.4.1 signalR file and applied the same changes. Those changes are:

windows.jQuery changed to jQueryShim
swapped order of parameters for $.inArray (this one seems strange but I didn't investigate why, I just copied the change from the earlier version)
exports at the bottom of the file.

I also updated the readme to give a more full example of using it with Typescript and pointed people to the @types/signalr-no-jquery package that provides types.

fixes #47 and #49 and #50

erzki commented 4 years ago

I added a similiar PR now. What a timing!

How did you get this to work without adding support for headers in jqueryShim? I had to add that in my PR to get it to work.

gregveres commented 4 years ago

@erzki Hi Erik, I saw your fork and wanted to reach out to you, but github does not give us a way to communicate. I wanted to see if you were doing the same thing.

Now, I might have missed something in my PR. I tried to look through the changes in 2.4.1 to see if there was extra jquery use that needed some work. I didn't see any, and my version worked for my use. But I don't use the full extent of SignalR.

If your version is more complete, then by all means, let's use that.

I just hope that @DVLP is around and can produce a new npm version for us.

@erzki are you using TS? If so are you using the @types/signalr-no-jquery or are you doing something different? The @types package isn't really complete but it seems to be good enough for what I need so far.

erzki commented 4 years ago

Hi

We are using Typescript and the @types/signalr-no-jquery I have not looked into that actually, mostly focused on getting this to work with a hosted Azure SignalR Service.

But I know the types are missing som stuff. E.g. events on the Connection.

Den ons 15 apr. 2020 kl 05:34 skrev Greg Veres notifications@github.com:

@erzki https://github.com/erzki Hi Erik, I saw your fork and wanted to reach out to you, but github does not give us a way to communicate. I wanted to see if you were doing the same thing.

Now, I might have missed something in my PR. I tried to look through the changes in 2.4.1 to see if there was extra jquery use that needed some work. I didn't see any, and my version worked for my use. But I don't use the full extent of SignalR.

If your version is more complete, then by all means, let's use that.

I just hope that @DVLP https://github.com/DVLP is around and can produce a new npm version for us.

@erzki https://github.com/erzki are you using TS? If so are you using the @types/signalr-no-jquery or are you doing something different? The @types https://github.com/types package isn't really complete but it seems to be good enough for what I need so far.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DVLP/signalr-no-jquery/pull/51#issuecomment-613796373, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMIMEL7USGSNYDXVJ2ZE33RMUTMFANCNFSM4MG6IZVA .

erzki commented 4 years ago

Approved by @itaispector but this PR is not compatible with Azure SignalR Service. Or is it @gregveres ?

itaispector commented 4 years ago

I truly apologize, it was by mistake, I did not test if its compatible with Azure SignalR Service.

gregveres commented 4 years ago

PR #52 should be the one that is merged. I don't know if this one is compatible with Azure Service because I don't use Azure service for SignalR. Erik does so we should go with his PR instead of this one.

I would just recommend that you pull in Readme.MD from my PR because I updated it to reflect how to use this package with typescript (it includes PR #47 )

gregveres commented 4 years ago

Please include @erzki 's pull request #52 intsead of this one. I am closing this one to avoid any confusion.

MosheL commented 4 years ago

@erzki I was found a workaround for the missed events on connection.

The problem is the jq mock in the function triggerHandler was lossing events in some type of uses. specific I was able to see it when the events was recived before the connection was finished to connect.

so, I was wrote a hack as I didn't want to change the jquery mock itself. Feel free to use it in the correct way.

 connectingMessageBuffer: new ConnectingMessageBuffer(this, function (message) {
                function triggerHandler(event, args) { //jquery shim has a bug. bypass it
                    var _this = this;
                    var events = _this[0].events || {}; // <-- this is the workaround
                    var handlers = events[event] || [];
                    handlers.forEach(function (fn) {
                        if (args && args[0] && args[0].type === undefined) {
                            args = [{
                                type: event
                            }].concat(args || []);
                        } else {
                            args = args || [];
                        }

                        fn.apply(_this, args);
                    });
                }
                 $connection.triggerHandler = triggerHandler;
                 $connection.triggerHandler(events.onReceived, [message]);
            }),

DVLP / signalr-no-jquery

upgraded to SignalR 2.4.1 #51