Re: [w3c/IndexedDB] Add getAllRecords() method and update getAll()/getAllKeys() to support direction option (PR #461)

@evanstade approved this pull request.



> @@ -1371,6 +1371,42 @@ To <dfn>convert a value to a key range</dfn> with
 
 </div>
 
+
+A <dfn>potentially valid key range</dfn> is an ECMAScript value that is probably a [=key=] or [=key range=], however, [=convert a value to a key range|converting=] this value to a [=key range=] may throw an exception.

"probably" seems like a strange word to use here, granted it's hard to phrase this properly. Here are two other attempts though:

This is any value that has a _type_ which is convertible to a key range. Whether the specific _value_ successfully converts to a key range --- i.e. `[=convert a value to a key range=]` throws an exception on it --- is not relevant.

Let _s_ be the set of all types for which there exists at least one value of that type which does not cause an exception to be thrown when passed to `[=convert a value to a key range=]`. A <dfn>potentially valid key range</dfn> is an ECMAScript value for which the type is in _s_.

> @@ -1801,6 +1837,29 @@ store.put(4); // will throw DataError
 ```
 </div>
 
+<!-- ============================================================ -->
+## Record Snapshot ## {#record-snapshot-construct}

meganit: "Record snapshot" (i.e. s/S/s)

>  
-        If successful, |request|'s {{IDBRequest/result}} will
-        be an {{Array}} of the [=/keys=].
+        If successful, |request|'s {{IDBRequest/result}} will be an {{Array}} of the [=/keys=].
+
+    : |request| = |store| . {{IDBObjectStore/getAllRecords()|getAllRecords}}({|query|, |count|, |direction|})
+    ::
+        Retrieves the [=/keys=] and [=/values=] of [=object-store/records=].
+
+        The |query| option specifies a [=/key=] or [=key range=] to match.
+        The |count| option limits the number or records matched.
+        Set the |direction| option to "{{IDBCursorDirection/next}}" to retrieve the first |count| records, or "{{IDBCursorDirection/prev}}" to return the last |count| records.
+
+        If successful, |request|'s {{IDBRequest/result}} will be an {{Array}}, with each member being a {{IDBRecord}}.

meganit: "a**n** IDBRecord"

> @@ -3174,7 +3247,7 @@ the first existing key in that range.
 
 <div algorithm>
 
-The <dfn method for=IDBObjectStore>getAll(|query|, |count|)</dfn> method steps are:
+The <dfn method for=IDBObjectStore>getAll(|query_or_options|, |count|)</dfn> method steps are:

nit: I think it's supposed to be queryOrOptions (see: `legacyOutputDidListenersThrowFlag`)

> +[Exposed=(Window,Worker)]
+interface IDBRecord {
+  readonly attribute any key;
+  readonly attribute any primaryKey;
+  readonly attribute any value;
+};
+</xmp>
+
+<div class="domintro note">
+    : |record| . {{IDBRecord/key}}
+    ::
+        Returns the record's [=key=].
+
+    : |record| . {{IDBRecord/primaryKey}}
+    ::
+        If the record is retrieved from an [=index=], returns the key of the record in the index's [=index/referenced|referenced object store=].

nit: maybe _was_ retrieved? Past tense seems better aligned with this being a snapshot i.e. a thing from the past, not live updating.

> +      ::
+          1. Let |key| be the result of [=/converting a key to a value=] with |record|'s key.
+          1. [=list/Append=] |key| to |list|.
+
+      : "value"
+      ::
+          1. Let |serialized| be |record|'s [=/value=].
+          1. Let |value| be [=ECMAScript/!=] [$StructuredDeserialize$](|serialized|, |targetRealm|).
+          1. [=list/Append=] |value| to |list|.
+
+      : "record"
+      ::
+          1. Let |key| be the |record|'s key.
+          1. Let |serialized| be |record|'s [=/value=].
+          1. Let |value| be [=ECMAScript/!=] [$StructuredDeserialize$](|serialized|, |targetRealm|).
+          1. Let |record snapshot| be a new [=record snapshot=] with its [=record snapshot/key=] set to |key|, [=record snapshot/value=] set to |value|, and [=record snapshot/primary key=] set to |key|.

should `|record snapshot|` be `|recordSnapshot|`?

> @@ -1371,6 +1371,38 @@ To <dfn>convert a value to a key range</dfn> with
 
 </div>
 
+
+A <dfn>potentially valid key range</dfn> is a value that is a [=key/type|type of key=] or a [=key range=].
+However, [=convert a value to a key range|converting=] a [=potentially valid key range=] to a [=key range=] could throw an exception.
+
+NOTE:
+  For example, a value that is a [=BufferSource/detached|detached BufferSource=] is a [=potentially valid key range=] that will throw an exception when used with [=convert a value to a key range=].
+
+<div algorithm>
+
+  To determine when a value <dfn>is a potentially valid key range</dfn> with |value|, run these steps:

I suppose it just feels like this is redundant, although I can see that it's hard to reuse the existing algorithm. Perhaps the existing algorithm could be changed to disintinguish between "invalid value" and "invalid type"? I dunno, up to you.

The other hairy part of this is coming up with a good name for "record snapshot". Can you remind me why we don't return different types from the two different getAllRecords methods, which would sidestep the need to define this unified type?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/w3c/IndexedDB/pull/461#pullrequestreview-2964219877
You are receiving this because you are subscribed to this thread.

Message ID: <w3c/IndexedDB/pull/461/review/2964219877@github.com>