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

Preventing cyclic dependency by inlining Op.iterator #58

Closed
wants to merge 2 commits into from

Conversation

stigi
Copy link
Contributor

@stigi stigi commented Aug 14, 2020

Fixes #57 by inlining Op.iterator.

This PR also extracts the iterator tests into their own file.

One question open question: Delta.Op.Iterator was part of the public API of this module, even though not mentioned in the Readme. It got replaced by Delta.Iterator, but not sure if this is considered a breaking change. I checked the main quill sources wether it's been accessed and also did a GitHub Search, and only one place turned up, so I guess we're pretty safe.

Testplan

npm run test
npm run lint

@stigi stigi changed the title Inlining Op.iterator Preventing cyclic dependency by inlining Op.iterator Aug 14, 2020
@wachunga
Copy link

Would love to see this merged. The circular dependency is causing me trouble when built with rollup.

@stigi
Copy link
Contributor Author

stigi commented Nov 17, 2020

@jhchen Let me know if I can do anything to help with this PR getting stamped 🙌

@jhchen
Copy link
Member

jhchen commented Sep 15, 2021

This has been merged 8c4d16e - thanks!

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.

Circular dependency between Op and Iterator
3 participants