chore: improve types for MapeoProject's kProjectReplicate
#900
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
*This change should have no user impact.* *I recommend reviewing this with whitespace changes disabled.* `MapeoProject`'s `kProjectReplicate` member will give type errors if called with a boolean. We aren't doing this today, but [we will in our upcoming server work][0]. This happened because TypeScript's `Parameters` utility type only takes the *last* signature of an overloaded function. To quote [the docs][1]: > When inferring from a type with multiple call signatures (such as the > type of an overloaded function), inferences are made from the last > signature (which, presumably, is the most permissive catch-all case). > It is not possible to perform overload resolution based on a list of > argument types. To fix this, we specify the types manually rather than pull them out of `Parameters`. Doing this fixes the potential type error. I also did a small amount of cleanup on the types in this function. (This change is primarily to fix an error in our upcoming server branch, but I think it's useful on its own.) [0]: https://github.com/digidem/comapeo-core/blob/9aaeb37910821a6779ec8eef88af995e94448ba3/src/mapeo-project.js#L140-L141 [1]: https://www.typescriptlang.org/docs/handbook/2/conditional-types.html
gmaclennan
approved these changes
Oct 23, 2024
There was a problem hiding this comment.
Looks good. Bear in mind that this now excludes from types passing a Protomux instance, which is supported, however the return type is different when passing a protomux. I don't think we currently use this method with a protomux instance (although we do use hypercore.replicate(protomux) in our sync API). This is an upstream bug, but I think our types are also incorrect (types say return type is Protomux, but I think the return type is NoiseStream).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change should have no user impact.
I recommend reviewing this with whitespace changes disabled.
MapeoProject'skProjectReplicatemember will give type errors if called with a boolean. We aren't doing this today, but we will in our upcoming server work.This happened because TypeScript's
Parametersutility type only takes the last signature of an overloaded function. To quote the docs:To fix this, we specify the types manually rather than pull them out of
Parameters. Doing this fixes the potential type error.I also did a small amount of cleanup on the types in this function.
(This change is primarily to fix an error in our upcoming server branch, but I think it's useful on its own.)