Skip to content

Conversation

@FZambia
Copy link
Member

@FZambia FZambia commented Jan 23, 2025

Support Headers Emulation introduced in Centrifugo v6.

This PR changing headers from Map<String, dynamic> to Map<String, String> in Client config. This is a breaking change, but I believe it must not cause huge troubles beyond a simple refactoring, I never seen someone using multiple values for the same header. If we find such a case - can bring dynamic back.

Also some refactoring to get rid of dart.html

// If you want to use Headers in web environment – make sure your headers use
// string values, centrifuge-dart will then automatically attach them to connect
// frame (using Headers Emulation feature).
headers: <String, dynamic>{'X-Example-Header': 'example'},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<String, Object?>{} better than <String, dynamic>{}

dynamic means "This can be everything, and you can call everything; do not check type"
Object? means "This is can be everything, but you should check the type before do anything"

Thats a bad practice to use dynamic anywhere, because you just omit any type checks, e.g.:

void main() {
  dynamic x = "string";
  x.toDouble(); // Exception during runtume
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try to move away from it and try using Map<String, String>, did in latest commit

url,
ClientConfig(),
ClientConfig(
headers: <String, dynamic>{'Authorization': 'example'},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<String, Object?>{'Authorization': 'example'},

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not actual with Map<String, String> anymore

Comment on lines 12 to 21
import 'web_in.dart' if (dart.library.js_interop) 'web_in.dart' as web;
import 'web_out.dart' if (dart.library.io) 'web_out.dart' as nonweb;

bool isWeb() {
try {
return web.isWebContext();
} catch (_) {
return nonweb.isWebContext();
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Правильный способ, если тебе просто нужна константа для понимания веб это или нет:

const bool kIsWeb = bool.fromEnvironment('dart.library.js_util');

суть способа в проверке доступность библиотеки js_util на этапе компиляции, тк она существует только для JS и WASM
этот способ использует Flutter SDK

также можно

const bool kIsWeb = identical(0, .0);

это точно будет работать в вебе при транспиляции Dart в JS, но есть сомнения на счет WASM
суть хинта в том, что для JS 0.0 и 0 это ссылка на один и тот же литерал num. а для всех остальных платформ это разные инстансы int и double
этот способ раньше до WASM использовал Flutter SDK

а если хочешь сделать через импорты, надо записать вместо:

import 'web_in.dart' if (dart.library.js_interop) 'web_in.dart' as web;
import 'web_out.dart' if (dart.library.io) 'web_out.dart' as nonweb;

так (тоесть именно в ЕДИНОМ импорте указывай "if"):

import 'vm.dart'
    // ignore: uri_does_not_exist
    if (dart.library.js_interop) 'js.dart';

и создать два файла vm.dart и js.dart
в каждом из которых методы и геттеры с одинаковыми сигнатурами, например чото типа того:

bool get isWeb => true;

Map<String, Object?> getPlatformMetadata() => <String, Object?>{
  'agent': window.navigator.userAgent
};

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want separate files for each platform you should do this way:

$ mkdir platform/
$ echo "const bool kIsWeb = true;" > platform/js.dart
$ echo "const bool kIsWeb = false;" > platform/vm.dart

and optionally create barrel file platform.dart:

export 'platform/vm.dart'
    // ignore: uri_does_not_exist
    if (dart.library.js_interop) 'platform/js.dart';

/*
import 'platform/vm.dart'
    // ignore: uri_does_not_exist
    if (dart.library.js_interop) 'platform/js.dart';

void main() {
  print(kIsWeb ? 'Web' : 'Not Web');
}
*/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Changed to use a single import with if


/// Set allows updating connection headers.
///
void setHeaders(Map<String, dynamic> headers);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Map<String, /* String | List<String> */ Object?> headers
or
Map<String, String> headers
or
Map<String, List<String>> headers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 216 to 219
@override
void setHeaders(Map<String, dynamic> headers) {
_headers = headers;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit safer option to make copy of headers, because user can pass mutable HashMap and mutate it over time.

  @override
  void setHeaders(Map<String, Object?> headers) =>
    _headers = Map<String, Object?>.of(headers);

or

@override
void setHeaders(Map<String, Object?> headers) => 
  _headers = <String, Object?>{
     for (final MapEntry(:String key, :Object? value) in headers.entries)
        if (value is String) key: value
        else if (value is Iterable) key: value.whereType<String>().join(', '),
  };

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 494 to 502
if (isWeb()) {
// Use headers emulation in web context.
final Map<String, String> emulatedHeaders = Map.fromEntries(
_headers.entries
.where((entry) => entry.value is String)
.map((entry) => MapEntry(entry.key, entry.value as String)),
);
request.headers.addAll(emulatedHeaders);
}
Copy link

@PlugFox PlugFox Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (kIsWeb) {
   // It would be a dead code and tree shaked at Dart VM (dart:io) platforms
   // Use headers emulation in web context.
   request.headers.addAll(<String, String>{
     for (final MapEntry(:String key, :Object? value) in headers.entries)
        if (value is String) key: value
        else if (value case Iterable iterable)
           key: iterable.whereType<String>().join(', ')
        /*
           or:
             key: iterable.firstOrNull?.toString() ?? '',
           or:
             for (final e in iterable.whereType<String>())
               key: e,
        */
  });
}

Copy link

@PlugFox PlugFox Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That work this way:

import 'dart:convert';

void main() {
  final headers = {
    '1': 'a',
    '2': ['b', 'c', 'd'],
    '3': 'e',
  };
  print(const JsonEncoder.withIndent(' ').convert(
    <String, String>{
     for (final MapEntry(:String key, :Object? value) in headers.entries)
        if (value is String) key: value
        else if (value case Iterable iterable)
           key: iterable.whereType<String>().join(', ')
        /*
           or:
             key: iterable.firstOrNull?.toString() ?? '',
           or:
             for (final e in iterable.whereType<String>())
               key: e,
        */
    }
  ));
  /*
    {
     "1": "a",
     "2": "b, c, d",
     "3": "e"
    }
  */
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx, not actual anymore

Comment on lines 1 to 2
// This file is only used in web environments
bool isWebContext() => true; // Always returns true in web contexts No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at suggestion before.

If you want separate files for each platform you should do this way:

$ mkdir platform/
$ echo "const bool kIsWeb = true;" > platform/js.dart
$ echo "const bool kIsWeb = false;" > platform/vm.dart

and optionally create barrel file platform.dart:

export 'platform/vm.dart'
    // ignore: uri_does_not_exist
    if (dart.library.js_interop) 'platform/js.dart';

/*
import 'platform/vm.dart'
    // ignore: uri_does_not_exist
    if (dart.library.js_interop) 'platform/js.dart';

void main() {
  print(kIsWeb ? 'Web' : 'Not Web');
}
*/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done I suppose

ClientConfig _config;
ClientConfig get config => _config;

Map<String, dynamic> _headers = {};
Copy link

@PlugFox PlugFox Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You no need default value here, if set it before constructors body:

ClientImpl(this._url, this._config, this._transportBuilder)
  : _token = _config.token,
    _data = _config.data,
    _headers = _config.headers;

Map<String, Object?> _headers;

or you can make it late and use constructors body:

ClientImpl(this._url, this._config, this._transportBuilder) {
    _token = _config.token;
    _data = _config.data;
    _headers = _config.headers;
}

late Map<String, Object?> _headers;

Comment on lines 115 to 119
ClientImpl(this._url, this._config, this._transportBuilder) {
_token = _config.token;
_data = _config.data;
_headers = _config.headers;
}
Copy link

@PlugFox PlugFox Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ClientImpl(this._url, this._config, this._transportBuilder)
  : _token = _config.token,
    _data = _config.data,
    _headers = _config.headers;

You can set it before constructor's body.
And optionally make a copy of hash map.

@FZambia FZambia marked this pull request as ready for review January 24, 2025 18:02
@FZambia FZambia merged commit 4c23682 into up_web_socket_channel Jan 25, 2025
4 checks passed
@FZambia FZambia deleted the headers_emulation branch January 25, 2025 06:39
FZambia added a commit that referenced this pull request Jan 25, 2025
* update web_socket_channel, min SDK 3.3
* Headers emulation and setHeaders method to update headers   (#92)
@FZambia FZambia mentioned this pull request Jan 25, 2025
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

Successfully merging this pull request may close these issues.

3 participants