Fixed circular dep #138
Fixed circular dep #138
Conversation
|
👍 |
|
I fought with this problem for a few frustrating hours, and this patch fixes it for me. 👍 |
|
Any update/feedback on this? |
|
@ryanzec There are some bug PR's I'd like to include first for the next release before adding this PR. I'm busy with a deadline at my work and I like to set some time aside for reflux releases in case things go awry with a release. Breaking bugs at release has happened before, so I'm cautious with the releases especially with refactorings such as these. Don't worry, I'd like to add this PR as soon as possible after the bugfix release. |
|
@spoike cool, sounds good |
|
Can't automatically merge. Would be nice if it did. :-) |
|
I'm all in favor of fixing circular dependencies, but not sure if moving everything into one file is the right way to go about it. Typically circular dependencies are a symptom of an architectural problem, and the risk is that moving everything into the same place just hides the symptom without actually fixing the issue. |
|
Well I don't know the codebase well enough to know how to change the architecture and that is something I am not going to invest time in as I have since move on with a different solution for a Flux system. If you want the PR, let me know, I will try to find some time in the next couple of weeks to fix it. If you think this solution is not the correct one, then you can disregard this PR. |
I have read that browserify can have issues with circular dependencies and I think the changes I have made here should resolve the issues I have experienced with #129.
The tests still all pass with these changes, the code can be compiled with browserify without any issues, and mocha tests for code that includes this library run without issues too.
These are mostly structure changes that prevent a file1 requires file2 and file2 requires file1 situations and I tried to following the coding standards that I saw. Please let me know if there is anything that I need to fix.