Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default types for Instance should not be any #83

Open
NickClark opened this issue Jul 5, 2017 · 5 comments
Open

Default types for Instance should not be any #83

NickClark opened this issue Jul 5, 2017 · 5 comments
Milestone

Comments

@NickClark
Copy link

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
Copy link
Member

notheotherben commented Jul 7, 2017

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
Copy link
Member

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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants