feat: Update useSubscribe to accept Replicache or Pick<Replicache, 'subscribe'> so it can accept ReflectClient #25
Conversation
…ubscribe'> so it can accepts ReflectClient
grgbkr
changed the title
src/index.ts
Outdated
|
|
||
| export function useSubscribe<R extends ReadonlyJSONValue>( | ||
| rep: Replicache | null | undefined, | ||
| rep: Replicache | Subscribeable | null | undefined, |
There was a problem hiding this comment.
Why do we need both Replicache and Subscribeable?
There was a problem hiding this comment.
Was debating this. I think it makes it more obvious to customers that they can pass Replicache here... it still not obvious that they can pass ReflectClient.
I think something like:
subscribable: Replicache | ReflectClient | null | undefined
might me more obvious.
| }, | ||
| "peerDependenciesMeta": { | ||
| "replicache": { | ||
| "optional": true |
| "peerDependencies": { | ||
| "react": ">=16.0 <18.0", | ||
| "react-dom": ">=16.0 <18.0", | ||
| "replicache": ">=4.0.1 <10.0 || >7.0.0-beta <7.0.0 || >8.0.0-beta <8.0.0 || >9.0.0-beta <9.0.0" |
There was a problem hiding this comment.
Do we know why did this package need replicache as a dependency in the first place? It seems like since we are not constructing it, all we should need is the type, and only then at dev time... So it seems like this should have worked with replicache as a devDependency and nothing else.
There was a problem hiding this comment.
It could have always been an optional peer dependency (in older versions of npm all peer dependencies were optional). But we still need the peer dependency to specify what versions of replicache this is compatible with.
| }); | ||
| } | ||
|
|
||
| export type Subscribeable = Pick<Replicache, 'subscribe'>; |
There was a problem hiding this comment.
What I was thinking was actually describing the subscribe interface. But now that I see maybe why you didn't want to do that - the subscribe interface is large and detailed. It could also fall behind head of either Replicache or Reflect.
What if instead we have this package just directly depend on Replicache and Reflect and have the argument be Replicache|Reflect.
I think that is fine, it's not going to add to the compile size, or even the install size for our users -- it just means when we work on this we install both packages.
|
#38 supersedes this. |
Also makes the
"replicache"peer dependency optional (using peerDependenciesMeta) so npm doesn't automatically install"replicache"when"replicache-react"is intalled.