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

Unable to upgrade connect-redis@6.1.3 to connect-redis@7.0.1 [typescript] #387

Closed
hvqzao opened this issue Apr 8, 2023 · 12 comments
Closed

Comments

@hvqzao
Copy link

hvqzao commented Apr 8, 2023

Hi,
First, I've read Release notes for 7.0.0, and I've attempted to migrate my code.
I'm removing @types/connect-redis@0.0.20, upgrading connect-redis to connect-redis@7.0.1 and making changes in code.
I can't make Typescript to stop complaining and just compile the code.
I see others are also struggling, so I've created simplest possible repo with this specific case in mind: https://github.com/hvqzao/cr-701
(first commit is the old version which works with no issues and second which does not).
The error I'm getting is:

Argument of type '{ client: Redis; }' is not assignable to parameter of type '(options?: SessionOptions | undefined)

Could you please tell me what I'm doing wrong?
Thank you & sorry for the hassle!

@wavded
Copy link
Collaborator

wavded commented Apr 10, 2023

The issue here is your are passing the session object into connect-redis. That is no longer required and will cause this error. See the migration notes for more details:

https://github.com/tj/connect-redis/releases/tag/v7.0.0

@wavded wavded closed this as completed Apr 10, 2023
@hvqzao
Copy link
Author

hvqzao commented Apr 10, 2023

No, I'm not passing session object to connect-redis, this line is commented out:

  // const RedisStore = ConnectRedis(Session)

The problem is located here:

    store: RedisStore({
      client: redis
    })

I'm sorry but release notes do not mention this issue. How can I address that?
Could you point out an example Typescript code which works with ioredis or share an update to the repo I've mentioned earlier?

@wavded
Copy link
Collaborator

wavded commented Apr 10, 2023

Ahh sorry, I must have looked at the wrong commit. The issue you have with the above code is you are not initializing the constructor with new. Should be new RedisStore({

@hvqzao
Copy link
Author

hvqzao commented Apr 10, 2023

Yes, that is also why I've commented out with new RedisStore({ as it caused different error.
RedisStore is defined as alias to a function:

(alias) function RedisStore(options: (options?: session.SessionOptions) => express.RequestHandler): s.RedisStore
(alias) namespace RedisStore
import RedisStore

The following code:

    store: new RedisStore({
      client: redis
    }),

will cause Typescript to fail with error:

'new' expression, whose target lacks a construct signature, implicitly has an 'any' type.ts(7009)

@wavded
Copy link
Collaborator

wavded commented Apr 10, 2023

Hmmm.. I cannot duplicate that error with your repo locally. The errors go away when I add new. That error makes me think you have still have the legacy @types/connect-redis installed or cached somehow.

@hvqzao
Copy link
Author

hvqzao commented Apr 10, 2023

No, I've tested it with fresh repo (I've commited the change replacing store: RedisStore({ with store: new RedisStore({:

git clone https://github.com/hvqzao/cr-701 connect-redis-issue
cd connect-redis-issue
npm install -E
npm start

tsc gives slightly different error than VSCode:

image

@wavded
Copy link
Collaborator

wavded commented Apr 10, 2023

Screenshot 2023-04-10 at 1 48 05 PM

Following your steps on a fresh checkout I get the following errors, this is due to it missing the @types/node @types/express and @types/express-session in the package.json. However, if I install those there, I get no errors:

Screenshot 2023-04-10 at 1 51 07 PM

@hvqzao
Copy link
Author

hvqzao commented Apr 10, 2023

That didn't work on my side. I'm facing this issue on multiple projects / environments / workstations. Do you have connect-redis linked locally on your workstation via npm link?

@wavded
Copy link
Collaborator

wavded commented Apr 10, 2023

No link locally (I wondered the same earlier and double checked). I am running this against the published 7.0.1 release and have tried on multiple environments as well with the same result. Hmm... have you been able to replicate on say a Docker container or something so we could try the same environment? Wish I could be of more help here...

@hvqzao
Copy link
Author

hvqzao commented Apr 12, 2023

Thanks for your time. I must say I see such thing for the first time. This repo works on one workstation, but results in error mentioned above on another ¯_(ツ)_/¯. Same version of node is used. I tried to reboot, remove ~/.npm, remove package-lock, node_modules, reinstall deps. The funny thing is that when vscode complains, npm start also complains. If it works in one, it works in both. I will keep looking into that. Or future release of connect-redis will solve the feature automagically just bacause of version bump.

@jackey8616
Copy link

@hvqzao I have same issue, but solve with removed @types/connect-redis.
I saw there is no @types/connect-redis install in your repo.
Maybe you accidentally installed it?

@hvqzao
Copy link
Author

hvqzao commented Apr 23, 2023

Hi @jackey8616 thank you for the support!
Well, the strange thing is that the issue got resolved by itself after some time (24h-48h) after applyling changes from 7.0.0 release notes to all my repositories that were using connect-redis. It could be that it was some caching issue. I don't know. What's important - right now it works 👍

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

No branches or pull requests

3 participants