#153: IndexedDB 2.0 features review

Visit on Github.

Opened Feb 17, 2017

Hello TAG!

I'm requesting a TAG review of:

Further details (optional):

You should also know that...

We have submitted tests covering all the features above to web-platform-tests. Chrome (with the Experimental Web Platform Features flag) passes all the test cases (159/159). Firefox (nightly) passes over 98.7% (157/159) of the test cases, and Safari (Tech Preview 23) passes over 99.3% (158/159) of the test cases.

Firefox has already shipped all the IndexedDB 2.0 features mentioned above, based on the Editor's Draft specification, in Firefox 51. Safari is about to ship the features above in Safari 10.1, which is included in the OS X 10.12.4 beta. We (Chrome) have implemented the features under the Experimental Web Platform Features flag, and plan to ship them following this TAG review.

We'd prefer the TAG provide feedback as (please select one):

  • leave review feedback as a comment in this issue and @-notify @pwnall and @inexorabletash

Please preview the issue and check that the links work before submitting

For background, some decent explainers:

https://github.com/w3c/ServiceWorker/blob/master/explainer.md https://github.com/zkoch/paymentrequest/blob/gh-pages/docs/explainer.md https://github.com/WICG/IntersectionObserver/blob/gh-pages/explainer.md (although this one includes IDL, which an explainer should not)

Discussions

Comment by @cynthia Feb 21, 2017 (See Github)

Blunt first pass comments from someone without much background on the design...

Just out of plain curiosity: since binary keys expect the underlying type information to be discarded when stored, would it be a problem if two different array buffers containing the same data at binary level work as the same key?

I'm not sure what the real world implications of this side effect would be (I suspect minimal) but it could in theory be a problem in extreme corner cases.

I've looked at IndexedDB/issues/#22 and the use case behind allowing renaming wasn't noted - what is the use case for this?

I need a bit more time to look at key generators, will post an update soon.

Comment by @inexorabletash Feb 21, 2017 (See Github)

...since binary keys expect the underlying type information to be discarded when stored...

There may be developer confusion that e.g. a Uint8Array and Float32Array consisting of the same backing bytes have the same value as a key. (This was touched on in the linked discussion). This pattern shows up in a few other places in the platform where APIs consume binary data; we eventually converged on the conventions specified in WebIDL's "get a copy of the bytes..." and the BufferSource abstract type. So the platform is consistent, but again there is the possibility of developer confusion.

...use case behind allowing renaming...

Most of the discussion around store/index renaming took place offline, and I failed to capture the intents there. (This also included guidance from @annevk on using setter rather than e.g. setName or anything awkard like that.)

This blog post by @bevis-tseng covers the use cases pretty well: https://hacks.mozilla.org/2016/10/whats-new-in-indexeddb-2-0/

...key generators...

Nothing (intentionally) new with key generators in 2.0 vs. 1.0, although we have identified a spec gap and interop issues with edge cases, discussed at https://github.com/w3c/IndexedDB/issues/147 - more eyes on everything welcome!

Comment by @pwnall Feb 21, 2017 (See Github)

Here is a rationale for renaming.

Applications that use the active record pattern have a 1:1 matching between stores and class names, for example Widget instances are persisted in a "widgets" store. When performing a code refactoring that renames the class, it is desirable to be able to rename the store as well, in order to maintain the pattern. Naming is hard, so the engineers of a 2-3-year old app are likely to be able to find a better name for at least one class with persistent instances.

I authored homework submission software that has been used for for 9+ years, and I've renamed a couple of models.

Comment by @cynthia Feb 22, 2017 (See Github)

Closed as LGTM as per discussed on 2/22 call.

Comment by @slightlyoff Feb 22, 2017 (See Github)

Per today's discussion, closing this with an "LGTM".

Thanks again to @pwnall for the thoughtful and detailed review request. In future, we'd love to see these features much sooner in the design phase. We don't bite!

Comment by @pwnall Feb 22, 2017 (See Github)

Thank you very much for the quick turnaround time!

@slightlyoff Point noted! I'll definitely get in touch sooner when designing features.