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

feat: relax dependency on Momento SDK #22

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

cprice404
Copy link
Contributor

While investigating this issue:

#18

I found that given our current example code it's fairly easy for
a user to get into a situation where they end up with two different
versions of our SDK dependency, and they get weird type errors and
can't compile.

The primary reason for this is that you need to construct a CacheClient
from the SDK in order to pass to the constructor for the Redis client
from this library. If you have typical eslint rules enabled, you
will be warned that you can't reference classes from the SDK such as
CacheClient without having an explicit dependency on the SDK in
your package.json. If, in attempting to resolve that problem, the user
does npm install @gomomento/sdk, and the version that gets installed
is not the exact same version that the momento-ioredis-client is pinned
to, then they will get build errors.

I think we should do two things to remedy this:

  1. relax this dependency version; I normally advocate for strict dependency
    versions but when you have to pass objects across API boundaries it
    becomes problematic.
  2. Re-export the CacheClient, Configuration, and any other objects from the
    sdk that users of this momento-ioredis library will need to pass to the redis
    client constructor; that way they can avoid having to add an explicit
    dependency on the momento sdk at all.

This commit does the first of those 2 items. In a subsequent PR we'll address
the second one.

While investigating this issue:

#18

I found that given our current example code it's fairly easy for
a user to get into a situation where they end up with two different
versions of our SDK dependency, and they get weird type errors and
can't compile.

The primary reason for this is that you need to construct a CacheClient
from the SDK in order to pass to the constructor for the Redis client
from this library. If you have typical eslint rules enabled, you
will be warned that you can't reference classes from the SDK such as
`CacheClient` without having an explicit dependency on the SDK in
your package.json. If, in attempting to resolve that problem, the user
does `npm install @gomomento/sdk`, and the version that gets installed
is not the exact same version that the momento-ioredis-client is pinned
to, then they will get build errors.

I think we should do two things to remedy this:
1. relax this dependency version; I normally advocate for strict dependency
  versions but when you have to pass objects across API boundaries it
  becomes problematic.
2. Re-export the CacheClient, Configuration, and any other objects from the
  sdk that users of this momento-ioredis library will need to pass to the redis
  client constructor; that way they can avoid having to add an explicit
  dependency on the momento sdk at all.

This commit does the first of those 2 items. In a subsequent PR we'll address
the second one.
@cprice404 cprice404 merged commit 9eef6f5 into main Apr 12, 2024
12 checks passed
@cprice404 cprice404 deleted the relax-momento-dep-constraint branch April 12, 2024 18:52
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.

2 participants