EventFlux icon indicating copy to clipboard operation
EventFlux copied to clipboard

Browser Support

Open Imgkl opened this issue 1 year ago • 9 comments
trafficstars

References

  • https://github.com/Zekfad/fetch_client/
  • https://github.com/dart-lang/http/issues/595
  • https://github.com/dart-lang/http/issues/337
  • https://github.com/flutter/flutter/issues/43343
  • https://github.com/stevenroose/dart-eventsource/issues/31

Imgkl avatar Jan 17 '24 22:01 Imgkl

The fetch_api package is intended for web use, which would explain why it's relying on dart:js. However, this becomes an issue when you try to run your Flutter app on platforms other than the web because the package is not compatible with non-web environments. Just by importing it in the package, seems to cause issue.

Possible Solutions:

  • Creating a new package eventflux_client_manager and do conditional imports based on platform
    • Pros
      • Sounds simple
    • Cons
      • overhead of managing two packages just for sake of web support.
  • Implement a solution from ground up for web
    • Pros
      • highly customisable to my liking and user needs
    • Cons
      • Heavy lift.
  • Explore Dio in favour of http

Imgkl avatar Jan 26 '24 23:01 Imgkl

Any updates on this issue ? It's very important to start using EventFlux. Thank you

The-RootCause avatar Apr 26 '24 15:04 The-RootCause

Hi, I'm fetch_client author.

Was checking usage of package and stumbled upon this issue. We've fixed it in last version, so you can now import client in VM (it will throw UnsupportedError is you try to instantiate it).

As a side note, when you use web only package (which uses js_interop or dart:html) you should do conditional import like in this example from package:http: https://github.com/dart-lang/http/blob/dd31e64685b03a9700cc99d7b7fc637dd60c4d3a/pkgs/flutter_http_example/lib/main.dart#L13-L14

Hope this will help to resolve this!

Zekfad avatar May 04 '24 17:05 Zekfad

@Imgkl Do I understand correctly, that the fetch_client simply needs to be updated? Could contribute that if you're not already on it. :)

I'm looking into using EventFlux and need browser support.

Peetee06 avatar Nov 21 '24 11:11 Peetee06

@Peetee06

Please give this a try. I couldn't get the condition import to work or the vm method.

I'm happy to merge at as soon as possible if the web support is working.

Imgkl avatar Nov 21 '24 12:11 Imgkl

@Imgkl good, will try it out tomorrow 👍

Peetee06 avatar Nov 21 '24 14:11 Peetee06

I tested browser support with fetch_client successfully. I used https://sse.dev/test to test the SSE stream.

We have two possible solutions:

1. Instantiating FetchClient inside the connect method on web

This would mean we need to allow users to pass some parameters to connect that the FetchClient has, so users can configure it as needed:

FetchClient({
    this.mode = RequestMode.noCors,
    this.credentials = RequestCredentials.sameOrigin,
    this.cache = RequestCache.byDefault,
    this.referrer = '',
    this.referrerPolicy = RequestReferrerPolicy.strictOriginWhenCrossOrigin,
    this.redirectPolicy = RedirectPolicy.alwaysFollow,
    this.streamRequests = false,
  });

This will require adding a set of enums to EventFlux that we can map to the ones of FetchClient. This includes writing tests for the correct mappings. It also means we need to update them whenever the API of fetch_client changes in the future. E.g. when an enum value is added or removed.

2. Replacing HttpClientAdapter with BaseClient and documenting the use of fetch_client

This is a little less seamless for users, because they need to add fetch_client to their direct dependencies and configure it themselves. They can then pass it to connect via the httpClient parameter. It's also a breaking change, because we need to change the httpClient type to BaseClient so we can actually pass in the FetchClient.


~I prefer solution 2, because it gives users control over the correct FetchClient parameters, while preventing EventFlux from becoming more complex and error prone because of mapping and passing the parameters through to the FetchClient constructor.~

After having realized we need to change HttpClientAdapter to BaseClient which is a breaking change and thinking more from the user's perspective, I think it's actually nicer to go with option 1.

What do you think about these options @Imgkl?

Peetee06 avatar Nov 25 '24 11:11 Peetee06

Option 1 feels more user-friendly choice, especially for DX. We do need to write tests for mapping and Ideally we should lock the fetch_client version so we have much control over stuffs, If they push breaking changes.

But that said, I was also worrying about the connect method params getting out of hand, if we have to support these enums and params.

FetchClient({
    this.mode = RequestMode.noCors,
    this.credentials = RequestCredentials.sameOrigin,
    this.cache = RequestCache.byDefault,
    this.referrer = '',
    this.referrerPolicy = RequestReferrerPolicy.strictOriginWhenCrossOrigin,
    this.redirectPolicy = RedirectPolicy.alwaysFollow,
    this.streamRequests = false,
  });

Maybe we can have a new model called WebConfig or something, to encapsulate these web specific configs. This would help keep the connect method clean and make the parameters more manageable. Your thoughts?

Imgkl avatar Nov 26 '24 22:11 Imgkl

I agree that this is better DX. I'll go ahead and implement it using WebConfig so there is only a single additional parameter.

Peetee06 avatar Nov 27 '24 13:11 Peetee06