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

fix: UserSession user type augmentation #54

Merged
merged 3 commits into from
Feb 16, 2024

Conversation

Gerbuuun
Copy link
Contributor

@Gerbuuun Gerbuuun commented Feb 12, 2024

Closes #38
Closes #31

It seems like the user type inside the UserSession interface cannot be augmented directly so I created a separate User interface. This does work.

Fixing the possibly undefined after using requireUserSession is achieved by setting the type explicitly to be defined. (we can do this because we check it to be defined right before returning it)

@Atinux
Copy link
Owner

Atinux commented Feb 13, 2024

What do you think of it @harlan-zw

Since you also opened #53

@Atinux Atinux changed the title Fix: UserSession user type augmentation fix: UserSession user type augmentation Feb 13, 2024
@Atinux
Copy link
Owner

Atinux commented Feb 13, 2024

I am afraid that this is a breaking change for types definition, right?

@harlan-zw
Copy link
Contributor

harlan-zw commented Feb 15, 2024

Seems like it's doing the same as mine, does this generate the correct implicit types for useUserSession with this change?

Should close one of the PRs since they're accomplishing the same thing.

Also, FWIW breaking change of types, the types are already broken and users would need to force type augmentation to use the module as is anyway.

@Atinux Atinux merged commit 1b99533 into Atinux:main Feb 16, 2024
3 checks passed
@Gerbuuun
Copy link
Contributor Author

Gerbuuun commented Feb 16, 2024

Seems like the return type of requireUserSession is compiled to any (and all other things as well).
So explicitly typing like @harlan-zw did is also required.
This is on me for forgetting to test the build.

His changes, including the one described below should work.

export async function requireUserSession(event: H3Event): Promise<UserSession & { user: User }> {
  const userSession = await getUserSession(event)

  if (!userSession.user) {
    throw createError({
      statusCode: 401,
      message: 'Unauthorized'
    })
  }

  return userSession as UserSession & { user: User }
}

I'll push the fixes to this branch. Seems like a new pr is needed (see #55)
You can decide to merge his branch or mine as it is his work. (be sure to add the changes described above)

(Sorry about this. Still learning about contributing...)

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.

Better typing of UserSession Cannot augment user in UserSession
3 participants