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

Explicit Return Type on getUserSession #379

Merged
merged 3 commits into from
Apr 7, 2025
Merged

Conversation

Tmmcmasters
Copy link
Contributor

This should fix the typescript issue where some users, including myself, are not seeing the extended properties of the interface UserSession. This should operate as normal.

… fix the issue with the UserSession not being readable by ts server.
@Tmmcmasters
Copy link
Contributor Author

Issue Link: #358

@Tmmcmasters
Copy link
Contributor Author

This is a very simple PR. I hope that we can get this approved rapidly.

@Tmmcmasters
Copy link
Contributor Author

I was able to pull this down and utilize this into a broken project. It fixes the type errors that we were receiving

@sandros94
Copy link
Contributor

sandros94 commented Mar 24, 2025

IMO it should also be added to setUserSession and replaceUserSession, omitting id.

@atinux WDYT?

@Tmmcmasters
Copy link
Contributor Author

Tmmcmasters commented Mar 24, 2025

IMO it should also be added to setUserSession and replaceUserSession, omitting id.

@atinux WDYT?

I wil update it if that is what is desired

@Tmmcmasters
Copy link
Contributor Author

@atinux @sandros94 Just wanted to poke a reminder to please look at this. This has been waiting for approval for two weeks

@atinux
Copy link
Owner

atinux commented Apr 1, 2025

Sorry for the delay, actually it should be working without having to explicitly set the types, see working example:

CleanShot 2025-04-01 at 12 22 00@2x

Don't you have this in your project?

Copy link

pkg-pr-new bot commented Apr 1, 2025

Open in StackBlitz

npm i https://pkg.pr.new/atinux/nuxt-auth-utils@379

commit: a68760b

@atinux
Copy link
Owner

atinux commented Apr 1, 2025

In that case I believe we should return explicit types for all the exported utils, would you be to do that?

@Tmmcmasters
Copy link
Contributor Author

In that case I believe we should return explicit types for all the exported utils, would you be to do that?

Let me take a look and see the scope of this. I might be able to do something like this

@Tmmcmasters
Copy link
Contributor Author

Tmmcmasters commented Apr 1, 2025

@atinux I updated all server exported functions related to session. Do you want me to update other things?

@Tmmcmasters
Copy link
Contributor Author

In that case I believe we should return explicit types for all the exported utils, would you be to do that?

Removed the comment to hide some code....

@sandros94
Copy link
Contributor

Sorry for the delay, actually it should be working without having to explicitly set the types, see working example:

[...]

Don't you have this in your project?

I might be wrong but I'm noticing something like a racing condition depending on the ts server version and OS used. And since the local project might be declaring their own User, UserSession this becomes even more evident on some platform like windows

@atinux
Copy link
Owner

atinux commented Apr 3, 2025

Could you try with:

npm i https://pkg.pr.new/atinux/nuxt-auth-utils@379

And confirm it fixes your issue?

@Tmmcmasters
Copy link
Contributor Author

Could you try with:

npm i https://pkg.pr.new/atinux/nuxt-auth-utils@379

And confirm it fixes your issue?

Running some testing.

@Tmmcmasters
Copy link
Contributor Author

@atinux I am receiving an error when trying to install this package;
npm error code UNABLE_TO_GET_ISSUER_CERT_LOCALLY npm error errno UNABLE_TO_GET_ISSUER_CERT_LOCALLY npm error request to https://pkg.pr.new/atinux/nuxt-auth-utils@379 failed, reason: unable to get local issuer certificate npm error A complete log of this run can be found in: /home/tmmcmasters/.npm/_logs/2025-04-03T12_12_02_437Z-debug-0.log

@Tmmcmasters
Copy link
Contributor Author

Tmmcmasters commented Apr 3, 2025

@atinux Removed ssl required and ran into a security policy issue so I might not be able to test that. However, whenever I published my fork to a our own npm registry and tested with that, I was able to get it to work and it resolved the Typescript issues that we were facing.

@Tmmcmasters
Copy link
Contributor Author

@atinux Let me know if the above testing was sufficient

@atinux atinux merged commit 6c2cb93 into atinux:main Apr 7, 2025
4 checks passed
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.

3 participants