appy-one / acebase-core

Core functionality for AceBase realtime database repositories
MIT License
9 stars 11 forks source link

Allow better types for DataReference and DataSnapshot #20

Closed futurGH closed 1 year ago

futurGH commented 2 years ago

DataSnapshot#val() always returns any, despite the fact that, given the existence of TypeMappings binding, a user will often know at compile time what type will be returned. I have a few suggestions for type-level changes with no runtime impact, apart from the 3rd option, listed roughly in order from least change to acebase-core to greatest internal change. I'd be happy to submit a PR for any of them, just wanted to get feedback first.

Option 1 (Generic-as-cast)

Workbench Example

const snapshot = await db.ref<MyClass>("foo/test").get();
const value = snapshot.val();
//    ^? - MyClass

Option 2 (User declaration merging)

Workbench Example

declare module "acebase" {
    interface BindingMap {
            "foo/*": MyClass;
    }
}
const snapshot = await db.ref("foo/test").get();
const value = snapshot.val();
//    ^? - MyClass

Option 3 (bind as assertion function)

Workbench Example

db.bindType("foo/*", MyClass);
const snapshot = await db.ref("foo/test").get();
const value = snapshot.val();
//    ^? - MyClass
appy-one commented 2 years ago

Thanks, that all looks great! I'll try out the code from the workbenches, great stuff!

appy-one commented 2 years ago

Sorry it's taking a while to get back to you, I've been working to port all last js bits of all acebase packages to typescript. I'm almost finished, so I'll take another look at this once everything is working well and published. Thanks again!

futurGH commented 2 years ago

No worries! Let me know what you think whenever you can find the time to take a look and I'd be happy to get to work on an implementation.

Bubz43 commented 2 years ago

I've personally been using something similar to option 2 and it seems to work quite well.

Minimal Example Playground

Throw any object interface at it and you are good to go, though you are lying about the shape of the DB data and have to keep in mind you need some other way actually guarantee correctness on initial creation/migrations.

futurGH commented 2 years ago

Totally agree, thanks for the example! With compile-time–only type checking like TypeScript's, there's always the risk that data won't line up with expectations, especially when it's being moved across file system boundaries as in the case of a DB. I've been using a version of option 2 in my own projects as well, and it's definitely best accompanied by a runtime function to assert that the received type is indeed what you're expecting.

futurGH commented 1 year ago

Hope you don't mind a quick bump @appy-one — I haven't been using AceBase as of late so if there's interest I'd love to submit a PR while the implementation is somewhere in my brain 🙂

appy-one commented 1 year ago

@futurGH yes please, you are more than welcome to! I think option 1 would be the best starting point for now, maybe option 3 at a later stage!