point-source / dart_ping

Multi-platform network ping utility for Dart
30 stars 13 forks source link

[Updated all packages to the last version. Added universal_io to support web platform. #41

Open guyluz11 opened 1 year ago

guyluz11 commented 1 year ago

Fix: #40

point-source commented 1 year ago

Thanks for the PR! What is the purpose of supporting web in this case? You can't ping local net devices from a browser so even if it builds, it won't actually function.

guyluz11 commented 1 year ago

I can think of several reasons that adding a web support may help us.

  1. Flutter is in the process of changing web-based programs from JavaScript to webassembly, this means that there's a chance pinging will work even for browser without changing anything if we support the platform.
  2. It use the exactly same functions and have the same functionality but on top of that it will show support in pub.dev page for one more platform. This means that this package will be more popular.
  3. My app is using this dependency and it is crashing on build to browser. I'm suspecting that fixing that will help more users by giving them the option to compile the app to the browser and see the errors as they go along (and catch them) and not just in the build process.
point-source commented 1 year ago

There are a handful of ways to send a ping request:

All the research I've done on this topic up till now suggests the following:

Given these deliberate limitations which are there to ensure the security of the host, it is not currently feasible to "ping" from any application running inside a web browser. Neither js or wasm is capable of this at this time.

Additionally, since this dart_ping library makes use of host system binaries (with the exception of iOS) to ping other systems, it does not contain any code capable of constructing an ICMP packet. Even if we managed to compile this library to js or wasm, it would be useless because there is no "ping binary for web" that it can call.

As for point 2 of your post, I intentionally do not want this package to show web support because of the reasons above. If this were to show support for web, it would technically be misinformation as far as pub.dev is concerned because this package does not and cannot function on web even if it compiles without error. I would rather not mislead the community on this point.

Regarding your app and web support, I would love for your app to support web and simply inform the user that the ping function is not available in web mode. It seems you should be able to make use of conditional imports to avoid importing this package when compiling for web (much as universal_io, etc do). That way, you can manage how your app deals with the feature incompatibility without affecting the specifications of this package.

Thoughts?

guyluz11 commented 1 year ago

Yep you are right

AI have searched and found the same result https://stackoverflow.com/questions/69120994/using-ping-class-in-blazor-webassembly

It seems like web assembly will not support ping from the reason that you mentioned. It was shocking for me because I hear a lot the phrase web apps will replace native desktop apps, but it seems that they miss this piece of knowledge.

I will remove universal_io from the pr.

guyluz11 commented 1 year ago

Removed universal_io from pr.

Pr contains only the relevant parts which are mostly updating the packages.

point-source commented 1 year ago

Thanks. Since most of these dependencies are not major version increments, they should be forward compatible. Are you having any dependency resolution issues at the moment? And if so, which deps?

guyluz11 commented 1 year ago

No dependency resolution issue at the moment.