Solving circular import hack

[olemartinorg]

These fixes should prevent problems with circular imports. I still get 14 warnings about circular imports in rollup (regardless of using the main commonjs plugin or the alternate one), but at least the emitted code seems to work fine in both now.

This fixes #443 and hopefully #430, although I haven't been able to reproduce the issue in webpack.

I also looked into extracting other circular usages (like totext.ts accessing RRule.FREQUENCIES), but that quickly produced quite a lot of changes. I can continue trying to weed out the rest of the circular import warnings if you want, just let me know.

---

Thanks for contributing to rrule!

To submit a pull request, please verify that you have done the following:

• [Y] Merged in or rebased on the latest master commit
• [Y] Linked to an existing bug or issue describing the bug or feature you're
  addressing
• [N] Written one or more tests showing that your change works as advertised

[tgrosinger]

I can confirm that it solves #443 for me.

[kibertoad]

@jakubroztocil Ping?..

[tgrosinger]

Any chance we can get this merged? It's difficult using an unpublished fork in my project.

@jakubroztocil or @olemartinorg?

[olemartinorg]

I would if I could. As for me, I don't really care that much anymore. I have stopped using the dependency that had this as a dependency in my project - so don't count on me for solving the other circular import problems.

[kibertoad]

I would be more than happy to help address remaining circular dependencies, as well as help maintaining project in general (we plan to start depending on it from node-schedule), but looks like current maintainers are MIA :(

[tgrosinger]

In the interim I published a package with this change so I can use it in my project easier

[tgrosinger]

I would if I could. As for me, I don't really care that much anymore. I have stopped using the dependency that had this as a dependency in my project - so don't count on me for solving the other circular import problems.

That's totally fair. Thanks again @olemartinorg for the work you did to get it to this point. It's been hugely helpful.

[doylio]

I've been trying to solve this all day! I hope this gets merged soon! Thanks @olemartinorg !

[davidgoli]

The rationale for this IIRC was to make nlp an optional dependency - I had in mind to eventually split it out into its own package. But this simplifies things for now, so we should merge it.

[davidgoli]

👍

[tgrosinger]

Hooray, thank you!

[doylio]

Amazing!!!

[tgrosinger]

@davidgoli would you also be able to publish a new release to NPM? I don't think I can use the changes in my project until a release has been made.

[davidgoli]

Published 2.6.8