redis-js icon indicating copy to clipboard operation
redis-js copied to clipboard

@upstash/redis import in file causing module to be imported dynamically

Open cathykc opened this issue 3 years ago • 17 comments

This is an issue as of 1.3.2-alpha.0 - fine for 1.3.0 (the version we've now pinned our application to).

Platform: Next.js 12.0.7

We have a very simple file (redis.ts):

import { Redis } from "@upstash/redis";
const redis = Redis.fromEnv();
export default redis;

We then import this in another file (import redis from "./redis";) - The redis object is now a promise that needs to be awaited before we can access the exported properties. This is leading us to believe that Next is interpreting something upstream as needing to be dynamically imported.

cathykc avatar Jun 13 '22 20:06 cathykc

Hey @cathykc, thank you for reporting this.

We recently found something similar in nextjs I believe. For some reason it didn't load environment variables properly, when creating such a utility redis.ts file. The fix was to just not do that and create a new Redis.fromEnv() instance instead of importing from the utility file, which doesn't cause any issues, as the sdk is completely stateless.

However it's not a perfect solution either. I will dig some more.

In the meantime, can you try upgrading to v1.6.0 and see if this behavior still exists?

chronark avatar Jun 14 '22 09:06 chronark

We discovered the error while using v1.6.1 so the problem definitely still exists! Keep us posted - we'll pin it at 1.3.0 for now.

cathykc avatar Jun 14 '22 23:06 cathykc

I will take a look at this on monday or tuesday. It should be fairly easy to reproduce, thanks to your code snippet and explanation above.

chronark avatar Jun 15 '22 20:06 chronark

Hey @cathykc,

I created an example but it works as expected.

I also tried using default exports, that sitll works.

Can you compare it with your code and let me know what's different?

chronark avatar Jun 20 '22 07:06 chronark

@chronark yes it is the same the issue I had but also it worked fine in a minimal repo!!

Ali-Hussein-dev avatar Jun 22 '22 18:06 Ali-Hussein-dev

repo here is an example of the bug @chronark

Ali-Hussein-dev avatar Jun 23 '22 09:06 Ali-Hussein-dev

Hmmm, it might be caused by getServerSideProps. I'll play around with it, thanks for sharing.

chronark avatar Jun 23 '22 09:06 chronark

Hey @Ali-Hussein-dev @cathykc I tried this again and it worked just fine.

git clone https://github.com/Ali-Hussein-dev/bug-reproduce.git
cd bug-reproduce
yarn add @upstash/redis@latest

UPSTASH_REDIS_REST_URL=... UPSTASH_REDIS_REST_TOKEN=.. yarn dev

Also tried it deployed on vercel and that works too

chronark avatar Jul 18 '22 12:07 chronark

@chronark so we should update @upstash/redis to the last version 1.9

Ali-Hussein-dev avatar Jul 18 '22 13:07 Ali-Hussein-dev

@Ali-Hussein-dev yeah please try.

chronark avatar Jul 18 '22 13:07 chronark

@Ali-Hussein-dev have you had time to try this yet?

chronark avatar Jul 28 '22 12:07 chronark

@chronark mmm, I tried it but unfortunately, it didn't work. I used Nx workspaces if that matters.

Ali-Hussein-dev avatar Jul 29 '22 08:07 Ali-Hussein-dev

Oh I remember I had plenty of issues with nx in the past. Do you have a minimal example you could share? otherwise I'll try creating one next week

chronark avatar Jul 29 '22 08:07 chronark

@chronark Repo with Nx, however, I recommend creating a new one with the last version of Nx.

Ali-Hussein-dev avatar Jul 30 '22 18:07 Ali-Hussein-dev

thank you so much. I'll check it out

chronark avatar Jul 30 '22 18:07 chronark

@Ali-Hussein-dev I'm confused, there's no mention of upstash or redis in that repo? Did you send the wrong one?

chronark avatar Aug 03 '22 10:08 chronark

@chronakr sorry you need to install it. npm i @upstash/redis and you are ready to go.

Ali-Hussein-dev avatar Aug 03 '22 13:08 Ali-Hussein-dev