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

Always define start and stop on Service #48

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EvanHahn
Copy link

Service's types say that start and stop can be undefined. This is impossible in practice because the only way to instantiate a Service is via Registry.prototype.publish, which always sets these properties.

This small refactor avoids this problem to ensure that start and stop are always defined, which should make the library a bit easier to use.

`Service`'s types say that `start` and `stop` can be undefined. This is
impossible in practice because the only way to instantiate a `Service`
is via `Registry.prototype.publish`, which always sets these properties.

This small refactor avoids this problem to ensure that `start` and
`stop` are always defined, which should make the library a bit easier to
use.
function start(service: Service, registry: Registry, opts?: { probe: boolean }) {
const service = new Service(config, start.bind(null, this), stop.bind(null, this))

function start(registry: Registry, { probe = configProbe }: { probe?: boolean } = {}) {
Copy link
Author

Choose a reason for hiding this comment

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

This uses config.probe, but lets you override it per call to start() as before.

@EvanHahn EvanHahn marked this pull request as ready for review January 22, 2024 23:28
EvanHahn added a commit to digidem/mapeo-core-next that referenced this pull request Feb 12, 2024
This version integrates all of our patches except the one for the
`probe` option when starting a `Service`. That version also has some
trickiness around types for `start` and `stop`, which are also fixed
here.

I submitted these patches [upstream] but haven't heard back as of this
writing.

[upstream]: onlxltd/bonjour-service#48
EvanHahn added a commit to digidem/mapeo-core-next that referenced this pull request Feb 13, 2024
This version integrates all of our patches except the one for the
`probe` option when starting a `Service`. That version also has some
trickiness around types for `start` and `stop`, which are also fixed
here.

I submitted these patches [upstream] but haven't heard back as of this
writing.

[upstream]: onlxltd/bonjour-service#48
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.

None yet

1 participant