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

fix: resolve circular import #465

Merged

Conversation

john-d-pelingo
Copy link
Contributor

@john-d-pelingo john-d-pelingo commented May 27, 2021

The RRule in rrulset.ts ends up being undefined because the root index is being
loaded while files such as nlp/totext.ts depend on the root index while
rrule.ts depends on nlp/index.ts. Then, at one point the execution
proceeds even though the root index has not fully loaded.


Thanks for contributing to rrule!

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

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

The RRule in rrulset.ts ends up being undefined because the root index is being
loaded while files such as nlp/totext.ts depend on the root index while
rrule.ts depends on nlp/index.ts. Then, at one point the execution
proceeds even though the root index has not fully loaded.
@john-d-pelingo
Copy link
Contributor Author

Screenshot 2021-05-27 at 11 37 44

A screenshot of the error where it points to the line export default class RRuleSet extends RRule in rruleset.ts.

@john-d-pelingo
Copy link
Contributor Author

john-d-pelingo commented May 27, 2021

Follow up to #444 regarding the circular imports.

@deanpienaar
Copy link

deanpienaar commented Jun 3, 2021

It would be awesome to have this merged/released

@onerzafer
Copy link

Any news on this one?. this seems to be the solution for rollup and vite builds

@dylanmcgowan
Copy link

Hello please merge this RRule does not work with vite.

@ralyodio
Copy link

ralyodio commented Mar 3, 2022

Can someone update or merge this? we need it for sveltekit.

@davidgoli
Copy link
Collaborator

This project used to have CI, where did it go?

Copy link
Collaborator

@davidgoli davidgoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to download & run tests manually since CI appears to have disappeared, but everything seems to work fine 👍

@davidgoli davidgoli merged commit 17268f4 into jkbrzt:master Mar 3, 2022
@john-d-pelingo john-d-pelingo deleted the jdp/fix/remove-circular-dependency branch March 3, 2022 23:44
@davidgoli
Copy link
Collaborator

@john-d-pelingo
Copy link
Contributor Author

@davidgoli Thank you for merging! Highly appreciated 🙏🏽

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.

6 participants