ExternalDartReference.toDartObject and Object.toExternalReference should support a null receiver

[leonsenft]

Both ExternalDartReference.toDartObject and Object.toExternalReference are defined as extensions on non-null types.

This poses a problem for generic APIs which could contain null:

extension type WritableSignal<T>(JSFunction _) implements Signal<T> {
  @JS('u')
  external void _update(JSExportedDartFunction update);

  void update(T Function(T) function) {
    _update(_jsCompatibleFunction(function).toJS);
  }

  /// Wraps [function] in a JS compatible function signature.
  ///
  /// We can't simply cast `T Function(T)` as `ExternalDartReference Function(ExternalDartReference)`.
  ExternalDartReference? Function(ExternalDartReference?) _jsCompatibleFunction(
    T Function(T) function,
  ) {
    return (value) {
      return function(_trustAs(value?.toDartObject))?.toExternalReference;
    };
  }
}

/// Returns [value] typed as [T] while omitting the `as` cast.
@pragma('dart2js:as:trust')
T _trustAs<T>(Object? value) => value as T;

Note the implementation of the wrapper function must use ?. to access both toDartObject and toExternalReference because T could be parameterized with a nullable type.

This causes dart2js to emit rather inefficient JavaScript:

  A.WritableSignal__jsCompatibleFunction_closure.prototype = {
    call$1(value) {
      var t1 = value == null ? null : value;
      t1 = this.$function.call$1(t1);
      return t1 == null ? null : t1;
    }
  };

If I'm not mistaken, isn't this simply equivalent to:

  A.WritableSignal__jsCompatibleFunction_closure.prototype = {
    call$1(value) {
      return this.$function.call$1(t1);
    }
  };

(Which could potentially be further inlined).

Could we simply redefine these extensions on Object? and ExternalDartReference?, removing the need for the ?. access?

  ExternalDartReference? Function(ExternalDartReference?) _jsCompatibleFunction(
    T Function(T) function,
  ) {
    return (value) {
      return function(_trustAs(value.toDartObject)).toExternalReference;
    };
  }
}

@srujzs

[srujzs]

I could see something like this working on the JS backends and avoiding the null-check (similar code for the other conversion):

extension ObjectToExternalDartReferenceExtension on Object? {
  ExternalDartReference? get toExternalReference => this as ExternalDartReference?;
}

Although it feels strange that we can't optimize t1 = value == null ? null : value to just t1 = value, which would presumably enable further inlining leading to the alternative code you suggested. It'd be nice if we can optimize that everywhere instead of just for these methods.

[leonsenft]

Your extension suggestion worked, thanks!

I'll use that as a workaround for now.

[leonsenft]

Closed by https://dart-review.googlesource.com/c/sdk/+/370663