point-source / dart_ping

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

dart_ping_ios should become dart_ping_flutter #28

Open guyluz11 opened 1 year ago

guyluz11 commented 1 year ago

If I understand correctly dart_ping_ios is the same package as dart_ping but includes flutter parts so that it can interact with flutter system calls like the package for ios.

But what if for example we want to add support for new features that need special android implementation as well as spacial ios implementation as well as Linux Windows Web...

Building a new package for each one will be a lot of projects to maintain.

We can do one project for flutter dart_ping_flutter and call the correct implementation by platform.

My suggestion is to change the project dart_ping_ios to dart_ping_flutter because it is already based on flutter.

point-source commented 1 year ago

It's been a while since I looked at that but I seem to recall it being such that you cannot use the dart_ping_ios package on it's own. I think it's just a plugin to dart_ping so it wouldn't be totally correct to rename it as you suggest.

That doesn't mean it couldn't be refactored / rebuilt to behave the way you mean. It would just be dart_flutter encapsulating dart_ping instead of the other way around. Perhaps this would make more sense but the reason I built it the way I did was so that one could use conditional imports to only import the ios / flutter side as necessary.

guyluz11 commented 1 year ago

I built it the way I did was so that one could use conditional imports to only import the ios / flutter side as necessary.

That could be handled inside the package and for the developers it will be easier and more convenient.

seem to recall it being such that you cannot use the dart_ping_ios package on it's own.

Why not just import dart_ping inside dart_ping_flutter / dart_ping_ios?.

But no need to do a big change if ios is the only one that needs to be supported.

git-elliot commented 10 months ago

but the reason I built it the way I did was so that one could use conditional imports to only import the ios / flutter side as necessary.

Can you tell me how to conditionally import dart_ping_ios? What I want is something like this (Hypothetically)

import 'package:dart_ping_ios/dart_ping_ios.dart'
    if (dart.library.io) 'package:network_tools_flutter/src/shim/dart_ping_ios.dart'
    if (dart.library.swift) 'package:dart_ping_ios/dart_ping_ios.dart';

I don't want to import your dart_ping_ios package on other platforms because it makes my package compatibility limited to android/ios which conversely supports desktop just like dart_ping.

What I think is dart.library.swift doesn't exists.. I saw on internet only io, html, and js are there but say if what you are saying about conditional imports is true then why didn't you do it in your dart_ping project for flutter_icmp_ping.

P.S.: I think dart.library.io represents all native platforms i.e., iOS, Android, Linux, Macos, and Windows and dart.library.html represents web

Source : https://stackoverflow.com/questions/66840203/flutter-conditionnal-import-mobile-vs-desktop

I guess I have to be satisfied with only Platform.isIOS wrapped everywhere I use your dart_ping_ios package

point-source commented 10 months ago

Ah yep, you're right. I didn't realize these conditionals were restricted only to dart.library.x directives. This would, as you've pointed out, interfere with the ability to conditionally import based on platform (ios/android vs desktop) rather than dart target (native vs web).

In the greater scheme, I currently cannot come up with any mechanism which allows me to support all flutter platforms and native (non-flutter) dart at the same time. I have to trade between non-flutter and iOS since the iOS implementation relies on flutter_icmp_ping. Once this issue gets resolved, this should become much easier to deal with since I can then bundle the swift/Obj-C code for iOS into a native (non-flutter) dart binary.

Perhaps you can try to override the detected platforms on your package as described here: https://dart.dev/tools/pub/pubspec#platforms

Is the issue that your package isn't being detected as having support even when it does or does it actually fail to work on desktop when you import the dart_ping_ios package? Note that DartPingIOS.register(); only needs to be called once and it only needs to be called before the first time you use dart_ping on an iOS platform. So simply using Platform.isIOS one time at application run to determine whether to call DartPingIOS.register(); should suffice to enable iOS support and still work on every platform other than web.

git-elliot commented 10 months ago

@point-source my package doesn't fail on other platforms because of dart_ping_ios, it just doesn't show platforms on pub.dev correctly. Thanks for the solution - I'll add in platforms section of pubspec.