search

DVLP / signalr-no-jquery

120 stars 78 forks source link

Upgrade to SignalR 2.4.1 #52

Closed erzki closed 4 years ago

erzki commented 4 years ago

To use a hosted Azure SignalR Service we need SignalR version 2.4.x.

This PR solves that.

malteA commented 4 years ago

I tried using your branch as a local npm package, with azure SignalR service. Maybe Im using it the wrong way for some reason I allways get status code 429 Connection count reaches limit. and i tried connecting only with 1 client, 1 server and 1 hub so i shouldnt run into the free trier limit with azure.

erzki commented 4 years ago

Hi

I have the same problem, but I can't see that it's because of my code.

/Erik

Den fre 17 apr. 2020 kl 15:29 skrev Malte notifications@github.com:

I tried using your branch as a local npm package, with azure SignalR service. Maybe Im using it the wrong way for some reason I allways get status code 429 Connection count reaches limit. and i tried connecting only with 1 client, 1 server and 1 hub so i shouldnt run into the free trier limit with azure.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/DVLP/signalr-no-jquery/pull/52#issuecomment-615244451, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMIMEMYR5JVCHRGXCHW7B3RNBKSXANCNFSM4MHSUX5Q .

malteA commented 4 years ago

Hi Erik,

your code seems to be fine, i fiddled around a lot and managed to get it working. I didnt need to change anything in your code.

In my Startup.cs i had to set the ConnectionCount = 1 afterwards it just worked.

app.MapAzureSignalR(this.GetType().FullName, new HubConfiguration(), options => options.ConnectionCount = 1);

Malte

erzki commented 4 years ago

Great that you found out about ConnectionCount. We are paying for the Azure Service anyway so I didn't spend time on that.

How do we proceed with this PR?

DVLP commented 4 years ago

thanks for your contributions! are there any breaking changes here?

erzki commented 4 years ago

It should not be any breaking changes by updating signalR javacsript version from v2.2.1 to 2.4.1.

We have tested it in our code base and it worked without changing any code.

It would be nice if someone else can try it aswell.

gregveres commented 4 years ago

@erzki @DVLP I will try it on my code code and get back to you. If Erik's code works, then I will be happy to use his PR instead of my PR.

In my PR, I have incorporated some changes to the readme file to update how to use this with typescript. I don't care if you don't take those changes since I already know how to use it with typescript now. I just need the official package to be upgraded to use the newest version of signalr.

gregveres commented 4 years ago

@erzki @DVLP It seems to work fine. I would appreciate it being merged and a new NPM version created.

I would also recommend you take the readme from my PR

erzki commented 4 years ago

@gregveres I copied your Readme file now into this PR (#51). Also added text that it works together with a hosted Azure SignalR Service.

gregveres commented 4 years ago

Ok. I will see how I can retract my PR so that it isn't confusing.

cmonteiro128 commented 4 years ago

@DVLP @erzki Can we get this merged?

alaaan commented 4 years ago

Would love to see this merged

erzki commented 4 years ago

I would like to get it merged but I don't have permissions. Should I create a new package?

cmonteiro128 commented 4 years ago

@erzki I would unless @DVLP responds soon

alaaan commented 4 years ago

@erzki hook up a new package! :)

erzki commented 4 years ago

Hi! Back after summer vacation.

What do you think the package should be named?

cmonteiro128 commented 4 years ago

Hi! Back after summer vacation.

What do you think the package should be named?

Be creative :)

alaaan commented 4 years ago

Hi! Back after summer vacation.

What do you think the package should be named?

Follow your heart!

gregveres commented 4 years ago

honestly I would name it signalr-no-jquery2 so that when people search for signalr jquery they find it.

erzki commented 4 years ago

Please try it https://www.npmjs.com/package/signalr-asp-net

e965 commented 4 years ago

Because @erzki disappeared somewhere, I made my own fork from the project with some improvements (and with the updated SignalR, of course).

enjoy https://github.com/cojamru/signalr-no-jquery

DVLP commented 4 years ago

Looks like I neglected this package for too long but I merged the PR and published in NPM too. As you can see I'm not too involved in this project anymore due to being at 100% capacity. I made this package on one contract job, a few years ago in a legacy system for my and my team's convenience and never thought it will be very useful in the future due to even at its creation time ASP Core already being available.