-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Package yrs binaries as Nuget packages #15
Comments
Hi, @goldsam! That would be definitely appreciated. This is already part of the plan since I didn't plan to put the burden of providing the Yrs binaries on the clients anyway, but this would only be relevant after finishing the bindings. |
It seems like your making amazing progress very quickly. I'll see what I can do |
@LSViana I've made a little bit of progress in generating and packaging the binaries. The more challenging part of this task is selection of the binary to load at runtime. I was going to take some inspiration from SkiaSharp and Tesseract (the later of which uses Interop.net internally) but wanted to first get your opinion before doing any work you might disagree with. |
@LSViana Following your feedback from #19 , my updated build against main of LSViana/y-crdt shows 4 tests failing (165 passing). Is that expected? Failed ObserveDeepHasPathWhenAdded [43 ms]
Error Message:
Expected: 1
But was: 3
Stack Trace:
at YDotNet.Tests.Unit.Arrays.ObserveDeepTests.ObserveDeepHasPathWhenAdded() in C:\Users\samgo\Documents\projects\ydotnet\Tests\YDotNet.Tests.Unit\Arrays\ObserveDeepTests.cs:line 61
Failed ObserveDeepHasPathWhenRemoved [1 ms]
Error Message:
Expected: 1
But was: 2
Stack Trace:
at YDotNet.Tests.Unit.Arrays.ObserveDeepTests.ObserveDeepHasPathWhenRemoved() in C:\Users\samgo\Documents\projects\ydotnet\Tests\YDotNet.Tests.Unit\Arrays\ObserveDeepTests.cs:line 128
Skipped Dispose [< 1 ms]
Skipped GetNewKeyReturnsNull [< 1 ms]
Skipped InsertDifferentTypeOnExistingKey [< 1 ms]
Skipped InsertEqualTypeOnExistingKey [< 1 ms]
Skipped InsertSameInstanceOnMultipleKeys [< 1 ms]
Failed LengthIsCorrectForAlphanumericAndEmojiCharacters [< 1 ms]
Error Message:
Expected: 10
But was: 8
Stack Trace:
at YDotNet.Tests.Unit.Texts.LengthTests.LengthIsCorrectForAlphanumericAndEmojiCharacters() in C:\Users\samgo\Documents\projects\ydotnet\Tests\YDotNet.Tests.Unit\Texts\LengthTests.cs:line 52
Skipped ObserveDeepHasPathWhenAdded [< 1 ms]
Failed ReturnsTheFullText [1 ms]
Error Message:
String lengths are both 7. Strings differ at index 6.
Expected: "Star. ?"
But was: "Star. ?"
-----------------^
Stack Trace:
at YDotNet.Tests.Unit.Texts.StringTests.ReturnsTheFullText() in C:\Users\samgo\Documents\projects\ydotnet\Tests\YDotNet.Tests.Unit\Texts\StringTests.cs:line 21
Skipped CreateAndApplySnapshot [< 1 ms]
Skipped CreateAndApplySnapshot [< 1 ms] |
That looks great to me! I see you were able to build for multiple platforms successfully and that's nice. I think we can work on downloading the correct binaries for the running platform & architecture during the build step from the published artifacts, right? If the file is already downloaded, we just skip not to slow down every build. I don't have lots of experience with that, to be honest, but what you started there is promising and if I can help with it after I'm done with the library bindings, I'll definitely try. 🙂
I just noticed that some tests are failing on Windows and, no, it's not expected. I've been mixing my development time between Unix and Windows machines to ensure the bindings are working on both sides but I let these issues go unnoticed. The issues with the The issues with emojis are probably due to the usage of UTF-16 characters because the Additionally, I talked to the maintainers of y-crdt and they mentioned that it probably makes sense to update the C# code to use UTF-16 by default when creating |
@goldsam An update here: the issues with emoji and other special characters should be fixed by now after the commits I pushed today. I tested them both on Windows and macOS and the tests are all passing now. Beyond fixing these 2 failing tests, I actually applied a library-wide update to read and write If you have any issues with these latest updates, please let me know. |
@goldsam are you still working on this? I would really like to use yjs in a .net project so I am pushing this and the server components. But I have almost zero experience with rust. |
@SebastianStehle Did you not see the link to my repo above? |
Same here, been looking for Yjs compatible port for C# that supports the current Yjs featureset. I also never worked with Rust and it's really a turn off having to struggle with that. A pipeline for this repository that builds NuGet packages with the necessary libaries would be amazing :) The pipeline looks good, but are there any packages? Or what's the status of it @goldsam? |
Ah I saw this one, but didn't realize that it also included the pipeline. Quite a huge PR. Thanks for the quick update :) |
Amazing, thank you guys! I'll be checking it out as soon as I got time. I'll support wherever I can :) I'll use it with SignalR, maybe I can provide a basic framework for people to use it without much work on their own if @SebastianStehle isn't doing this already. |
Hi, The reason is that I don't have the opportunity to battle test it at the moment. The question where to handle persistency and so on is challenging because the server is stateful. In general I think the best solution would be to use something like actors or grains where exactly on server stores a single document (e.g. with Microsoft Orleans), but this has other challenges around cluster management. So for my use case, where I have only very few people working with the yjs functionality I have decided to declare one node as yjs node. So I basically have N normal nodes and one yjs node (with a environment variable) and the UI connects to this dedicated node. |
Alright, fair points. Well for me it's not necessary as of now. I'm only building a prototype for now so no need for clustering yet. Is the websocket implementation compatible with Yjs' websockets (looks like it from what I saw)? |
Yes, it is. It uses the same encoders and stuff from y-protocols, And in the demo I use the normal websocket provider: It does not implement auth yet, but the y-websocket provider also does not support it. |
Well no auth is an issue, but also something I can ignore for now. Shouldn't be too hard to implement, though. Something like that: yjs/y-websocket#7 (comment) |
No auth is not entirely correct. You can implement the normal auth, but I think some headers are not supported for web sockets and stuff like this, therefore it can make sense to implement in the websocket part directly and not outside. |
Consumption of this work (upon release) would be easier if the Yrs binaries were provided as nuget packages. If you would like, I could put together a PR for this.
The text was updated successfully, but these errors were encountered: