dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.22k stars 1.57k forks source link

Arguments corresponding to JS property names in JS utilities should be nullable #45219

Open greglittlefield-wf opened 3 years ago

greglittlefield-wf commented 3 years ago

Dart SDK version: 2.12.0

For reference, the JS utilities from dart:js_util in question:

bool hasProperty(Object o, Object name);
dynamic getProperty(Object o, Object name);
dynamic setProperty(Object o, Object name, Object? value);
dynamic callMethod(Object o, String method, List<Object?> args);

The name argument in hasProperty/getProperty/setProperty and the method argument in callMethod should be Object? instead of Object, since null is a valid property name in JavaScript.

The following code currently compiles and runs correctly in dart2js, even though there are analysis errors:

import 'dart:js_util';

main() {
  final object = newObject();

  // Error: The argument type 'Null' can't be assigned to the parameter type 'Object'.
  setProperty(object, null, 'foo'); 

  // Error: The argument type 'Null' can't be assigned to the parameter type 'Object'.
  print(getProperty(object, null)); // prints "foo"
}

...and has the output I'd expect:

    function() {
      var object = {};
      object[null] = "foo";
      H.printString(H.S(J.toString$0$(object[null])));
    }
greglittlefield-wf commented 1 year ago

I did a little more research, and apparently in JS, when you're using null as a key, it actually gets coerced to a string first.

So, my above statement of "null is a valid property name in JavaScript" is incorrect, since property names must be strings; sorry about that!

Source: the ES6 spec:

Example illustrating this behavior in JS:

let object = {};
object[null] = 'test';
Object.keys(object); // ['null']
object[null];   // 'test'
object['null']; // 'test'

So one workaround to this issue is to use the string 'null' instead of null.

For instance, in a function that takes in a potentially nullable name:

void addProperty(Object object, Object? name) {
  setProperty(object, name ?? 'null', 'foo');
}

That gets roughly gets compiled to:

var t1 = name == null ? "null" : name;
object[t1] = 'test value';

which I wouldn't expect to add significant overhead.

Given that, perhaps this issue could be closed?