search

SierraSoftworks / Iridium

A high performance MongoDB ORM for Node.js
http://sierrasoftworks.github.io/Iridium/
Other
570 stars 25 forks source link

Default types for Instance should not be `any` #83

Open NickClark opened 7 years ago

NickClark commented 7 years ago

Enjoying Iridium so far. One big issue. The Instance typings include a [name: string]: any; which results in really surprising issues in tooling. For example, I can type any method/property name I want, and TS happily accepts it and assumes it's an any. This makes for tough refactors and I can no longer trust my tooling.

notheotherben commented 7 years ago

Hi Nick, Sorry that it's taken me a while to get back to you on this, I've been trying to recall why that functionality was added in the first place and I believe it was originally intended to cover cases where your schema may present a property which is not necessarily present on your Instance definition, or where you opt to use Iridium.Instance without sub-classing it.

I absolutely agree that it's a sub-optimal solution and it definitely has no place in the code-base from a maintainability and tooling perspective, however it is, unfortunately, part of the public API which does complicate any discussion about removing it. I'm going to leave this issue open as a reminder to myself that this should be removed in v8, however I'm afraid I can't give you a timeline on when that will happen.

PS: Apologies for closing out this issue and the linked commit, that was meant to be for #84

notheotherben commented 7 years ago

Hi @NickClark,

I've just put together the first round of a v8 alpha which includes this change. I didn't see any breaking effects on the examples or tests I have as part of the Iridium codebase, but as with all public API changes, this may break things. As you mentioned, that will almost certainly be for the better, however it's something to be aware of.

In addition to your change, this also fixes some issues with the TypeScript 2.4 compiler's new strict generic checks (which can be disabled for Iridium v7 by using the --noStrictGenericChecks compiler flag).

You can upgrade to the latest version of the alpha by running the following:

npm install --save iridium@next

Please have a look and let me know if anything unexpected breaks for you, I'd ideally like to get this to a stable state as soon as possible, but before I do so it'll need some real world testing to ensure it doesn't cause problems for anybody else.

Kind regards, Benjamin

NickClark commented 7 years ago

Thanks @SPARTAN563! Ya, this bug has been biting us hard. Our project is in early prototype, and refactoring has become a pain. This should resolve the issue nicely! I'll let you know how it goes!

NickClark commented 7 years ago

So far so good. Updating to @next immediately helped us find about a dozen bugs we had still managed to miss in the refactor, thanks so much!

notheotherben commented 7 years ago

I'm very glad to hear that @NickClark, please let me know if you encounter any issues with @next and I'll try to get them resolved as quickly as possible.