Add typings for JSRecord and some unsafe extensions for JSObject #487

nex3 · 2025-12-10T02:07:11Z

This doesn't include JSObject extensions that require types that
aren't defined yet, like JSSymbolicRecord.

This doesn't include JSObject extensions that require types that aren't defined yet, like JSSymbolicRecord.

js_interop/lib/unsafe.dart

js_interop/lib/src/unsafe/object.dart

kevmoo · 2025-12-10T02:32:32Z

We should do better on those JS failures in Wasm: dart-lang/sdk#62218

srujzs · 2025-12-11T00:52:47Z

js_interop/lib/src/record.dart

+  /// The operation is equivalent to doing `this[key] = value` for each key and
+  /// associated value in [other]. It iterates over [other], which must therefore
+  /// not change during the iteration.
+  void addAllRecord(JSRecord<V> other) => addPairs(other.pairs);


tiny nit: maybe addAllFromRecord instead?

srujzs · 2025-12-11T01:02:17Z

js_interop/lib/src/unsafe/object.dart

+@JS('Object.values')
+external JSArray<JSAny?> _values(JSObject object);
+
+/// Additional instance methods for the `dart:js_interop` [interop.JSObject]


nit: remove interop.

srujzs · 2025-12-11T01:07:18Z

js_interop/lib/src/unsafe/object.dart

+  /// See [`Object.entries()`].
+  ///
+  /// [`Object.entries()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries
+  List<(String, JSAny?)> get entries => [


Thoughts on making this a generic method instead so that the second tuple member can be a generic (and therefore avoid the need for a cast list)?

srujzs · 2025-12-11T01:16:59Z

js_interop/lib/src/unsafe/object.dart

+  /// The [name] must be a [JSString] or a [JSSymbol].
+  ///
+  /// [`Reflect.get()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/get
+  R getPropertyWithThis<R extends JSAny?>(JSAny name, JSAny? thisArg) =>


I feel like this and setPropertyWithThis are sufficiently different in name that we may want a Reflect type instead, especially if we're planning to add more Reflect members. Thoughts?

srujzs · 2025-12-11T01:19:02Z

js_interop/test/record_test.dart

+    );
+  });
+
+  test("addPairs()", () {


nit: did you mean to name this as clear()?

srujzs · 2025-12-11T01:20:02Z

js_interop/test/record_test.dart

+    expect(record.containsKey("foo"), isTrue);
+    expect(record.containsKey("baz"), isFalse);
+  });
+


nit: test for containsValue

srujzs · 2025-12-11T01:21:24Z

js_interop/test/unsafe/object_test.dart

+    () =>
+        expect(object.getPropertyWithThis("foo".toJS, object), equals(1.toJS)),
+  );
+


nit: test for getOwnProperty

kevmoo · 2025-12-11T04:48:12Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces JSRecord as a map-like wrapper for JS objects and adds several unsafe extensions for JSObject. The implementation is generally good, but I've found a few areas for improvement, mainly related to performance in JSRecord and some minor issues in documentation and tests. My review includes suggestions to optimize method implementations and fix typos.

js_interop/lib/src/record.dart

js_interop/lib/src/unsafe/object.dart

js_interop/test/record_test.dart

js_interop/test/unsafe/object_test.dart

nikeokoronkwo · 2025-12-15T18:32:18Z

Thank you for providing this! It's something I've really wanted officially in Dart JS Interop.

I wanted to ask why we only make use of JSRecord with string keys. According to the ES Spec, JS Records can have a string, number or symbol as a key, the same way a Dart Map can have any Dart type. Packages like zod, reduxjs and some others depend on this functionality, and it would be nice to ensure we do not prevent such packages from having undefined behavior (or just not work well at all) in Dart, or prevent users from being able to use such functionality (I currently use this functionality when working with OpenAPI docs for instance, and iterating through error code indices).

I'm mainly saying this because eventually, I'd want to replace the generated types we make using the web generator to these ones, so it'd be nice if these types could match rather than having to remake the same types.

nex3 · 2025-12-15T18:44:31Z

JS objects can't actually have numeric keys; numbers are coerced to strings. You can test this by running the following in the dev console:

const obj = {};
obj[1] = 1;
obj["1"]

Symbols are another matter. I plan to bring over another type, JSSymbolicRecord, which uses JSAny as its key type in place of String. I think it's valuable to have access to both, because there are many places where only strings are expected or allowed, and it's much simpler to work with these types when you don't have to manually wrap and unwrap JSString every time. Then we can use JSSymbolicRecord for places where the higher generality is allowed.

nikeokoronkwo · 2025-12-15T19:08:46Z

JS objects can't actually have numeric keys; numbers are coerced to strings.

Agreed, but numeric indexes could also still work, and people/js package authors still rely on this functionality anyways.

The main issue I'm having is that the packages I mentioned use custom types (usually unions, but there are instances of enums as well, which are usually numbers underlying) as indices, which do reduce to string eventually, but at compile time it isn't so clear. Especially with the way dart interop code is generated at the moment, such types, if unions, would show as JSAny, and would therefore not fit in such cases (even if we ignored the type, users may want to index with such values rather than having to coerce directly into a string/number).

Symbols are another matter. I plan to bring over another type, JSSymbolicRecord, which uses JSAny as its key type in place of String. I think it's valuable to have access to both, because there are many places where only strings are expected or allowed, and it's much simpler to work with these types when you don't have to manually wrap and unwrap JSString every time. Then we can use JSSymbolicRecord for places where the higher generality is allowed.

In that case, would it be fine if we have JSRecord as is (maybe with a warning or something else not to use this directly but to use the child types), and then have a JSStringRecord and JSSymbolicRecord inheriting from JSRecord, which the majority of users would use? That way, packages that don't use strings for indexing can still work, and the majority of people can still use.

nex3 · 2025-12-15T19:32:45Z

Agreed, but numeric indexes could also still work, and people/js package authors still rely on this functionality anyways.

In principle, you can pass any value to an object key and it'll work—but it'll also coerce the value to a string. This is the way of a loosely-typed dynamic language. But JS interop generally tries to frame the APIs as strictly as possible, and a JS object can't meaningfully hold any keys that aren't strings or symbols. Since other key types will be coerced anyway, it's easy enough to do that coercion eagerly and pass them to the string API.

It's also worth noting that the JSRecord API simply can't work for other types because they can't be symmetric on input and output. You may be able to store a number or other object in an object's key, but when you go to retrieve or list it you'll always get a string.

In that case, would it be fine if we have JSRecord as is (maybe with a warning or something else not to use this directly but to use the child types), and then have a JSStringRecord and JSSymbolicRecord inheriting from JSRecord, which the majority of users would use? That way, packages that don't use strings for indexing can still work, and the majority of people can still use.

The substantial majority of record-style object uses in JS are specifically string keys (I count about 75% Record<string, ...> versus any other Record<...> declaration in TypeScript, and much of the latter seems to be union types with string representations anyway), and as such it makes sense for that to be the default type named JSRecord than for the two types to have symmetrical names.

srujzs · 2025-12-16T05:33:02Z

@nikeokoronkwo Could the number/enum use-case be better handled if we had extra members that can set/update/etc. numeric keys? Since the numeric keys are stringified, should APIs like entries still return a Iterable<MapEntry<String, V>>? I agree with Natalie that the symmetry is off, but if we think it may make the numeric case easier if we had some members that operated on inputting numeric keys, that's an easier task. And unlike symbols, numbers get stringified so it seems like a natural fit too.

Re: the polymorphic option, we could have a base class for JSSymbolicRecord that would handle the numeric case too, but I don't think we should have the string version inherit from it as we would be forced to use JSString or do the conversions, which is pretty unideal.

Add typings for JSRecord and some unsafe extensions for JSObject

7efd84d

This doesn't include JSObject extensions that require types that aren't defined yet, like JSSymbolicRecord.

nex3 force-pushed the jsrecord branch from ba4e49c to 7efd84d Compare December 10, 2025 02:08

kevmoo reviewed Dec 10, 2025

View reviewed changes

js_interop/lib/unsafe.dart Show resolved Hide resolved

kevmoo reviewed Dec 10, 2025

View reviewed changes

js_interop/lib/unsafe.dart Show resolved Hide resolved

kevmoo reviewed Dec 10, 2025

View reviewed changes

js_interop/lib/src/unsafe/object.dart Show resolved Hide resolved

nex3 added 3 commits December 10, 2025 15:10

Document js_linterop/lib/unsafe.dart

92338e7

Fix WASM test

f8e3e72

Reformat

eec8dc5

srujzs reviewed Dec 11, 2025

View reviewed changes

gemini-code-assist bot reviewed Dec 11, 2025

View reviewed changes

Fix lints

6a29a0a

Add typings for JSRecord and some unsafe extensions for JSObject #487

Are you sure you want to change the base?

Add typings for JSRecord and some unsafe extensions for JSObject #487

Uh oh!

Conversation

nex3 commented Dec 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevmoo commented Dec 10, 2025

Uh oh!

srujzs Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

srujzs Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

srujzs Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

srujzs Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

srujzs Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

srujzs Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

srujzs Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

kevmoo commented Dec 11, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nikeokoronkwo commented Dec 15, 2025

Uh oh!

nex3 commented Dec 15, 2025

Uh oh!

nikeokoronkwo commented Dec 15, 2025

Uh oh!

nex3 commented Dec 15, 2025

Uh oh!

srujzs commented Dec 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants