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

Make interfaces depend only upon abstractions #14

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

sdaves
Copy link
Contributor

@sdaves sdaves commented Jan 27, 2018

Hello,

Using the iamacommandhandler interface currently depends on having a hard reference to DomainService and all of its dependencies. This pull request aims at making interfaces standalone for the DomainService by introducing an IDomainService, and interfaces for its dependencies. I've also added Symbols of the same name in each interface file, which allows inversifyjs injection during runtime, since typescript compiles away interfaces but keeps symbols.

Let me know if you feel this fits well with the project and I can write some tests to validate these new abstractions.

Thanks!

@sdaves sdaves changed the title WIP: Make interfaces depend only upon abstractions Make interfaces depend only upon abstractions Jan 27, 2018
@MichalPaszkiewicz
Copy link
Owner

Symbol is only available in EcmaScript6... do you have any idea if there is a way to compile it with backwards-compatibility?

@sdaves
Copy link
Contributor Author

sdaves commented Jan 27, 2018

IE11 is the only everygreen browser not currently supporting native symbols (shame shame).
https://kangax.github.io/compat-table/es6/#test-Symbol

Just add the following polyfill for IE11 support: import "core-js/es6/symbol";

@MichalPaszkiewicz
Copy link
Owner

Is there any chance you could fix the conflict with the latest code (I realised I hadn't chucked everything to github last time I pushed to npm) + add the polyfill? I'm rather stuck for time right now.

@sdaves
Copy link
Contributor Author

sdaves commented Jan 28, 2018

Fixed the conflict and added the polyfill :)

@sdaves
Copy link
Contributor Author

sdaves commented Jan 28, 2018

Added the optional getViewByName parameter to the handle function in IAmACommandHandler. Extracted IView to an interface too.

@sdaves
Copy link
Contributor Author

sdaves commented Jan 31, 2018

Do you see any more merge errors on your side?

@sdaves
Copy link
Contributor Author

sdaves commented Feb 3, 2018

When do you think you would be free to provide some feedback?

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

Successfully merging this pull request may close these issues.

2 participants