Test BigInt as keys and values in IndexedDB#10977
Test BigInt as keys and values in IndexedDB#10977inexorabletash merged 2 commits intoweb-platform-tests:masterfrom
Conversation
BigInt and BigInt wrappers are supported in serialization, per whatwg/html#3480 This support allows them to be used as IndexedDB values. However, BigInt is not supported as an IndexedDB key; support has been proposed in the following PR, but that change has not landed at the time this patch was written w3c/IndexedDB#231
inexorabletash
left a comment
There was a problem hiding this comment.
Thanks for the submission!
IndexedDB/bigint_value.htm
Outdated
|
|
||
| function invalidKey(key, name) { | ||
| async_test(t => { | ||
| createdb(t).onupgradeneeded = t.step_func(e => { |
There was a problem hiding this comment.
It's easier to just use e.g. indexedDB.cmp(0, key) - this will throw if the key is not valid. No need to create a database.
IndexedDB/bigint_value.htm
Outdated
| async_test(t => { | ||
| t.step(function() { | ||
| assert_true(test(value), | ||
| "condition does not apply to initial value"); |
There was a problem hiding this comment.
Assertion messages should be phrased in the form of the expected behavior, e.g. "Predicate should return true for the initial value"; the error output is confusing if the inverse is used.
IndexedDB/bigint_value.htm
Outdated
| // This support allows them to be used as IndexedDB values. | ||
|
|
||
| let i = 0; | ||
| function value(value, test, name) { |
There was a problem hiding this comment.
Can you rename the test argument to test_func or predicate? Since test is a testharness function it's confusing to see it re-used as an argument.
IndexedDB/bigint_value.htm
Outdated
| // This support allows them to be used as IndexedDB values. | ||
|
|
||
| let i = 0; | ||
| function value(value, test, name) { |
There was a problem hiding this comment.
Using value as both the name of the function and the argument is confusing. How about value_test for the function?
IndexedDB/bigint_value.htm
Outdated
| .createObjectStore("store") | ||
| .add(value, 1); | ||
|
|
||
| e.target.onsuccess = t.step_func(function(e) { |
There was a problem hiding this comment.
Arrow function instead of function ?
IndexedDB/bigint_value.htm
Outdated
| .transaction("store") | ||
| .objectStore("store") | ||
| .get(1) | ||
| .onsuccess = t.step_func(function(e) |
There was a problem hiding this comment.
Arrow function instead of function ?
IndexedDB/bigint_value.htm
Outdated
| @@ -0,0 +1,77 @@ | |||
| <!DOCTYPE html> | |||
| <meta charset="utf-8"> | |||
| <title>Values</title> | |||
There was a problem hiding this comment.
Provide a more informative title
IndexedDB/bigint_value.htm
Outdated
| <script src="support.js"></script> | ||
|
|
||
| <script> | ||
| // BigInt and BigInt wrappers are supported in serialization, per |
There was a problem hiding this comment.
Use "objects" instead of "wrappers" to be consistent with the usage below? Or use "wrappers" everywhere.
IndexedDB/bigint_value.htm
Outdated
| // https://github.com/whatwg/html/pull/3480 | ||
| // This support allows them to be used as IndexedDB values. | ||
|
|
||
| let i = 0; |
e1282cd to
f49bd54
Compare
|
implemented the requested changes, and added a missing .valueOf() call |
inexorabletash
left a comment
There was a problem hiding this comment.
Looks great, thanks!
Test BigInt serialization in IndexedDB values, and check that BigInt values cannot be used as IndexedDB keys. These are @littledan's tests with some changes suggested in reviews; see PR #9667 for the original version.