dstore icon indicating copy to clipboard operation
dstore copied to clipboard

Convert Store.js module and tests to TypeScript

Open kfranqueiro opened this issue 10 years ago • 2 comments

Convert src/Store.js to TypeScript as src/Store.ts, and tests/unit/Store.js as tests/unit/Store.ts.

In order to run unit tests with the changes, any instances of Store will need to be changed to Store.default in other JS modules until we convert them as well.

See #130 for general information on the conversion. If you have questions, comments, or thoughts for this module beyond the conversion itself, you can leave comments on this issue.

kfranqueiro avatar Jul 08 '15 18:07 kfranqueiro

This is going to depend on https://github.com/dojo/core/pull/31 being merged, since it uses lang.mixin. I should probably get that out of the way first.

kfranqueiro avatar Jul 08 '15 18:07 kfranqueiro

Comments for future consideration:

  • We should reevaluate how much of an opinion Store has on models, and how much it tries to do itself regarding their instantiation/use. I've been on one project so far where we attempted to use dstore with models, and we ended up overriding _restore more often than not. (Also, the use of __proto__ in here has irked me for as long as the code has existed.)
  • Either way, we will need to pull out reliance on any declare-specific constructs. (@maier49 will be pulling out the last two Store tests that are currently specific to that, since testing a "declare-like" store had necessitated some awkward code in the test module.)
  • (via @maier49 in the PR): What would be more efficient, executing QueryMethod once each time a store is instantiated or executing it each time store.filter is called? I think it's nicer to have this as a method than a property but I don't know if that justifies the extra function calls.
  • May want to revisit the tests invoking _setIdentity directly after we convert Memory - we may be able to remove them here without sacrificing coverage, if we test Memory both with/without objects with set methods

kfranqueiro avatar Aug 05 '15 21:08 kfranqueiro