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

Signal finder #404

Open
wants to merge 59 commits into
base: main
Choose a base branch
from
Open

Signal finder #404

wants to merge 59 commits into from

Conversation

pvh
Copy link
Member

@pvh pvh commented Jan 9, 2025

A signals-lite based implementation of the new approach to find(). I think this is now on a pretty promising path it:

  • can return a value synchronously if it's available (check the current state of the signal)
  • supports many listeners
  • handles unassisted progress (whoops on the generator)

The signals implementation is and is intended to be quite minimal but it probably needs a few more minor features. Eliminating manual deregistration would be one goal, and progressing in "epochs" to avoid glitches in the matrix would be the other. Cycle detection was a good idea. I may yet cave and adopt / vendor something small into the library but this isn't meant to be a broadly adopted signals library, just a small thing that we can adapt into the many other frameworks and libraries in the ecosystem.

Some notes: as of this writing there are still about a third of the repo tests failing. If this direction continues to work I'll probably move the find() logic into a standalone file so that it can be more easily inspected / tested. I'd also like to try updating DocHandle to emit one of these signals (at least optionally) as the document evolves.

I'd also like to reduce the duplication between dochandle and the find code in terms of state tracking as part of the cleanup but that might mean some adjustment of the current soon-to-be-deprecated sync protocol and I'm avoiding that can o' worms.

@pvh pvh requested a review from alexjg January 9, 2025 09:00
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.

1 participant