upstash / redis-js

HTTP based Redis Client for Serverless and Edge Functions
https://docs.upstash.com/redis
MIT License
668 stars 51 forks source link

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

Closed cathykc closed 1 year ago

cathykc commented 2 years ago

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.

chronark commented 2 years ago

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?

cathykc commented 2 years ago

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.

chronark commented 2 years ago

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 commented 2 years ago

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?

Ali-Hussein-dev commented 2 years ago

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

Ali-Hussein-dev commented 2 years ago

repo here is an example of the bug @chronark

chronark commented 2 years ago

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

chronark commented 2 years ago

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

Ali-Hussein-dev commented 2 years ago

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

chronark commented 2 years ago

@Ali-Hussein-dev yeah please try.

chronark commented 2 years ago

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

Ali-Hussein-dev commented 2 years ago

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

chronark commented 2 years ago

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

Ali-Hussein-dev commented 2 years ago

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

chronark commented 2 years ago

thank you so much. I'll check it out

chronark commented 2 years ago

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

Ali-Hussein-dev commented 2 years ago

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