Skip to content
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

WebAssembly Compilation #503

Open
NotTsunami opened this issue Dec 18, 2023 · 15 comments
Open

WebAssembly Compilation #503

NotTsunami opened this issue Dec 18, 2023 · 15 comments

Comments

@NotTsunami
Copy link
Contributor

Hi Steve. Thanks for the work on the MQTT client implementation. Something pretty exciting Flutter/Dart is working on is compilation to WebAssembly. It's something I've kept my eyes on from a spectator perspective, but definitely want to leverage once it's production-ready. It is still very early into the process, but Flutter 3.16 is the start of the journey and begins using the pkg:web package (master branch should be used for testing, though). It would be really cool if we saw a few package upgrades to support WASM compilation.

I believe that the work on this roughly boils down to:

  • Move mqtt_client from dart:html to pkg:web. It would roughly look like this.
  • Upgrade to http 1.1.2
    • The major version upgrade isn't breaking to this library itself, but sigv4 breaks as it cannot upgrade to http 1.0.0 or higher. This requires the move from sigv4 to aws_signature_v4. aws_signature_v4 is a Dart library provided by the AWS Amplify team, so it does upgrade the deprecated third-party library to a maintained first-party library, which is nice.

There is a little bit of work involved in the transition, and we might also want to wait for dart-lang/web@4cb5811ed06 to be included in an official pkg:web release to avoid manually adding the WebSocketEvents extensions like I did in the example commit, which has yet to happen. Flutter pins pkg:web 0.3.0 in Flutter 3.16, so the next version bump isn't likely until the next major release of Flutter, which is some months out as well. The roadmap on this change is likely relatively far, but it is something to start considering, I think. Happy to contribute where I can!

@shamblett
Copy link
Owner

Yes your right, the earlier we look at these things the better, I'll have to perform the same upgrade on some of my other packages so we might as well start here.

I've not looked into this area as yet, I'll familiarise myself with pkg:web but you seem to have done a lot of the legwork here already, thanks, happy to look at any pull requests from you.

As for sigv4 its only used in the AWS example. Its nice to have this but if we can't get the aws_signature_v4 package to work I can live without it, we can't let stuff like this stop us upgrading to newer packages.

@awvreenen95
Copy link

Hi @shamblett. I've recently raised an issue on the flutter github repo.

In the issue raised, I mentioned that the WebSocket factory method (from inside universal_html) was throwing an error. "mkustermann" Stated in his reply that the universal_html package needs to be migrated to use their static JS interop APIs and the conditional imports need to be updated.

Version 10.0.2 included the change from using dart:html to using the universal_html package.

@NotTsunami
Copy link
Contributor Author

NotTsunami commented Feb 16, 2024

Hi @shamblett. I've recently raised an issue on the flutter github repo.

In the issue raised, I mentioned that the WebSocket factory method (from inside universal_html) was throwing an error. "mkustermann" Stated in his reply that the universal_html package needs to be migrated to use their static JS interop APIs and the conditional imports need to be updated.

Version 10.0.2 included the change from using dart:html to using the universal_html package.

It doesn't really make sense for universal_html to migrate to pkg:web as it is intentionally a drop-in for the html package. The patch I linked in the original comment should serve as a much better starting point for you in migrating to pkg:web for WebAssembly purposes. IMO, we should just migrate universal_html->pkg:web ourselves and skip that process. We were using html directly until last month. You'll see a conflict if you try to merge it over the main branch but just replace universal_html with web.

@shamblett
Copy link
Owner

Yep Ok, we need universal_html for now as it fixes some flutter apk and web bundling problems as stated in the implementing PR #509, we can work out what to do with this when we get to implement this issue.

@NotTsunami
Copy link
Contributor Author

Yep Ok, we need universal_html for now as it fixes some flutter apk and web bundling problems as stated in the implementing PR #509, we can work out what to do with this when we get to implement this issue.

We don't need universal_html. We can prevent build errors with conditional imports: https://dart.dev/interop/js-interop/package-web#conditional-imports

@shamblett
Copy link
Owner

Yep OK, the original poster said this in the PR -

 it is currently not possible to bundle both browser and APK support within the same application.

Ok, I don't know enough about flutter to know if we can achieve the same thing using conditional imports, I would have though the OP would have looked at this though.

@NotTsunami
Copy link
Contributor Author

NotTsunami commented Apr 3, 2024

Yep OK, the original poster said this in the PR -

 it is currently not possible to bundle both browser and APK support within the same application.

Ok, I don't know enough about flutter to know if we can achieve the same thing using conditional imports, I would have though the OP would have looked at this though.

In a production environment, I currently ship mqtt_client from a monorepo with windows + web targets, and I have zero issues when I use conditional imports. I would also like to think that OP has looked at this, but I do not believe this is the case. I also do not believe it is the responsibility of mqtt_client to handle non-browser targets within MqttBrowserClient.

In any case, I opened a draft PR that should support WebAssembly compilation with Flutter 3.19.x as well as Flutter master.

@shamblett
Copy link
Owner

Yes, the browser client should only do what it claims to do, i.e. work in the browser.

The more I think about this the more I think we should what we need to do here then see where we are and see what if any effect this has had on other users, we can fix any problems that arise on a case by case basis when we(I) have a better understanding of them.

@naychrist
Copy link

Hi all. Thanks for the great mqtt client have gotten A LOT of use out of it :) Wondering if any new updates or plans for wasm? I was planning to migrate projects to wasm but still not able to get mqtt_client 10.5.1 to work with flutter 3.24.3. cheers.

@NotTsunami
Copy link
Contributor Author

Hi all. Thanks for the great mqtt client have gotten A LOT of use out of it :) Wondering if any new updates or plans for wasm? I was planning to migrate projects to wasm but still not able to get mqtt_client 10.5.1 to work with flutter 3.24.3. cheers.

I opened a draft pull request where I was working on moving it to WebAssembly, but the project I was using it for got shelved, so I haven't had a lot of opportunity to continue with the PR. At the very least, I believe the PR was compiling, but it was throwing exceptions.

@naychrist
Copy link

ok thanks for the work, I'll keep watching this space for updates. very excited for wasm in flutter but mqtt a must for me in most projects.

@shamblett
Copy link
Owner

@NotTsunami , Hi Tyler, looking forward, the contract I've been working on for a while ends this week so I'm going to have a lot more time on my hands to look at stuff like this. if your'e busy I can take this over if you are OK with this of course.

@NotTsunami
Copy link
Contributor Author

@NotTsunami , Hi Tyler, looking forward, the contract I've been working on for a while ends this week so I'm going to have a lot more time on my hands to look at stuff like this. if your'e busy I can take this over if you are OK with this of course.

Sure! That would be greatly appreciated.

@shamblett
Copy link
Owner

Just adding this ref for wasm compilation.

@shamblett
Copy link
Owner

The updates described above have now been implemented, the main one being the update to use package web. I have done my testing on this and re released the package at version 10.6.0. This is to garner any feedback from users as to any side effects upgrading to package web has introduced. This forms the basis for eventual compilation to wasm, this will come later when Dart fully supports it so this issue will be left open.

I also took the opportunity to upgrade several outdated deps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants