Update AuthUser to offer a better DX #1915

infomiho · 2024-03-21T14:45:21Z

Note

This is probably need to be a breaking change to make the API clean.

infomiho · 2024-03-24T22:30:59Z

waspc/data/Generator/templates/sdk/wasp/server/_types/index.ts

@@ -88,20 +87,5 @@ type Context<Entities extends _Entity[]> = Expand<{
 {=# isAuthEnabled =}
 type ContextWithUser<Entities extends _Entity[]> = Expand<Context<Entities> & { user?: AuthUser }>

-// TODO: This type must match the logic in auth/session.js (if we remove the


This TODO is now resolved yay! Since the type and the runtime value are based on the same logic.

infomiho · 2024-03-24T22:32:09Z

waspc/src/Wasp/Generator/AuthProviders.hs

@@ -43,3 +46,13 @@ emailAuthProvider =
    { E._providerId = fromJust $ makeProviderId "email",
      E._displayName = "Email and password"
    }
+
+getEnabledAuthProvidersJson :: AS.Auth.Auth -> Aeson.Value


Generating this JSON in one place instead of three.

infomiho · 2024-03-24T22:32:43Z

waspc/data/Generator/templates/sdk/wasp/auth/user.ts

+) {
+  const { auth, ...rest } = user
+  const identities = {
+    {=# enabledProviders.isEmailAuthEnabled =}


Generating fields based on the enabled auth methods.

Martinsos

Took a quick look, looks good in general, left some comments!
But I will let @sodic do a proper review.

How would you describe the crux of this PR regarding its impact on the user? getFirstProviderId is now method on user. You also have createAuthUser which helps with something from what I saw. Something else? This is only part of the Auth DX improvements right?

Martinsos · 2024-03-26T20:59:25Z

waspc/data/Generator/templates/sdk/wasp/auth/user.ts

+  return {
+    ...rest,
+    identities,
+    getFirstProviderUserId: () => getFirstProviderUserId(user),


Aha, so this is now method on the user!

Martinsos · 2024-03-26T21:03:14Z

waspc/examples/todoApp/src/apis.ts

 import express from 'express'

 export const fooBar: FooBar = (_req, res, context) => {
-  const username = getFirstProviderUserId(context?.user) ?? 'Anonymous'
+  const username = context.user?.getFirstProviderUserId()


Ok cool this is nicer now!
What does getFirstProviderUserId promise? If I have multiple providers, it will return id from one of them. And it will be consistent in the order, right? What if I add a new provider? Maybe this ID will become different, right? That might be ok,I am just curious.

It will return the first available provider :D the order might change with if you add a new provider.

I predict that this method will be mostly used by people who only have one provider anyways or just want to display something identifying the user. For proper "production usage" I suspect people will want to put some sort of an "email" or "username" on the User object anyways.

Yeah, I agree! We should make sure to mention this in the docs of this function, to make it clear how it is intended to be used, and what the pitfalls might be, and why they should therefore potentially put stuff on User instead. I don't think this stuff will be clear to a new Wasp user.

I mentioned putting stuff on User here: https://github.com/wasp-lang/wasp/pull/1915/files#diff-1da5418e699dcbebea8ab516c4e60235efd7191abad17057916ca5f9bba2ddb1R263

But I didn't comment on the pitfalls of the getFirstProviderUserId method.

waspc/examples/todoApp/src/pages/ProfilePage.tsx

sodic

I'm a big fan of this change!

Left a couple of comments to discuss.

sodic · 2024-03-27T16:07:20Z

waspc/data/Generator/templates/sdk/wasp/auth/useAuth.ts

@@ -3,7 +3,7 @@ import { deserialize as superjsonDeserialize } from 'superjson'
 import { useQuery, addMetadataToQuery } from 'wasp/client/operations'
 import { api, handleApiError } from 'wasp/client/api'
 import { HttpMethod } from 'wasp/client'
-import type { AuthUser } from './types'
+import type { AuthUser } from 'wasp/auth'


We're phasing out package imports in the SDK (as of very recently 😄), so let's not add new ones:

Context: #1922

sodic · 2024-03-27T16:08:25Z

waspc/examples/todoApp/src/dbSeeds.ts

-  }
-
-  return newUser
+  return createAuthUser(newUser as any)


This is something the users will have to do, I assume?

If that's the case, can we avoid the assertion?

I wanted to get this code to work, I wanted to make it work without an assertion but it proved difficult. I'll give it an another try.

I've now hidden this API since it would expose some server imports on the client. Now this test is written a bit differently with the {} as AuthUser method which works well for these kinds of tests.

waspc/examples/todoApp/src/pages/ProfilePage.tsx

sodic · 2024-03-27T16:12:33Z

waspc/examples/todoApp/src/webSocket.ts

@@ -8,7 +7,7 @@ export const webSocketFn: WebSocketDefinition<
  InterServerEvents
 > = (io, context) => {
  io.on('connection', (socket) => {
-    const username = getFirstProviderUserId(socket.data.user) ?? 'Unknown'
+    const username = socket.data.user?.getFirstProviderUserId() ?? 'Unknown'


How will we decide whether we want methods or getters? Is there a rule of thumb in the JS world?

Getters are nicer, and look less bureaucratic, but if some contain arguments, then the API looks janky.
Also, what if one of the getters that doesn't need any arguments today ends up needing arguments in the future?

Then again, getters are nicer and look less ~~like java~~ bureaucratic.

I was using a rule of thumb: if it's a property that they would be able to get just by going through the raw DB entity, I put a getter. If it's something special like getting the first available provider, I used a function.

The options seem to be:

user.getIdentities().email

user.identities.email

user.firstProviderId

user.getFirstProviderId()

The combination felt okay to me, as user.firstProviderId feels off since the jobs of getting the first provider id feels dynamic.

Yeah I like this logic, but I do have to admit I am also not quite used to using JS getters, so I can imagine somebody possibly convincing me the other way. But with knowledge I have, I like it.

sodic · 2024-03-27T17:22:56Z

waspc/examples/todoApp/src/user.ts

+  if (user.identities.google !== null) {
+    return `Google user ${user.identities.google.id}`


Much better!

infomiho · 2024-04-09T10:06:01Z

Docs are ready for review. I've tried to rewrite them to fit the new API and explain some extra stuff.

Martinsos

@infomiho I gave it a look, looks fine in general! Seems like there was no big structural changes, you mostly covered the change in the API + added the migration instructions?
It can be a bit hard to review docs sometimes, even harder than code, because in code I can really on Typescript / Haskell to know that stuff is vibing together, and I can somehow easier get a bigger picture, in docs it is a bit harder to figure out what was supposed to change, what is the context of the rest of the docs, ... . What might be helpful when doing docs change like this is if you wrote a tiny bit about what you changed, from the very high level, to give me an idea of what I am looking at, what should I focus on. You did that somewhat "rewrite ... new API ... explain extra stuff", so maybe that is it, not sure, but for example I would to hear if you want me to look at something specific, maybe what is that new stuff, ... .

web/docs/auth/_accessing-user-data-note.md

web/docs/auth/entities/_email-data.md

web/docs/migrate-from-0-13-to-0-14.md

Martinsos

@infomiho responded to comments!

Signed-off-by: Mihovil Ilakovac <[email protected]>

…object.

Signed-off-by: Mihovil Ilakovac <[email protected]>

infomiho force-pushed the update-auth-user-api branch from 39f81eb to 316bc30 Compare March 22, 2024 08:48

infomiho commented Mar 24, 2024

View reviewed changes

Martinsos reviewed Mar 26, 2024

View reviewed changes

sodic reviewed Mar 27, 2024

View reviewed changes

infomiho marked this pull request as ready for review April 4, 2024 11:17

infomiho force-pushed the update-auth-user-api branch from b03e3ff to 6e843a4 Compare April 4, 2024 11:17

infomiho requested review from Martinsos and sodic April 9, 2024 10:05

Martinsos reviewed Apr 10, 2024

View reviewed changes

Martinsos reviewed Apr 12, 2024

View reviewed changes

infomiho force-pushed the update-auth-user-api branch from 474c5fa to 1e9969b Compare April 15, 2024 10:12

infomiho added 16 commits April 22, 2024 16:55

Update AuthUser API

c7d3811

Signed-off-by: Mihovil Ilakovac <[email protected]>

Updates types. Cleanup.

4e0547f

Fixes types

ccf9dce

e2e tests

87ae042

Signed-off-by: Mihovil Ilakovac <[email protected]>

Updates headless tests

e704c2f

Split user.ts file. Improve types. Combine ID and data into a single …

babbf29

…object.

Update tests

3a8f82c

Signed-off-by: Mihovil Ilakovac <[email protected]>

Cleanup

7fb8247

Drops findUserIdentity

c3b1fa9

WIP docs change

9da36f5

Removes _rawUser property on AuthUser

b9528a0

Update docs on the new user API

689e17e

Fixes crud-testing not to leak password hash

40b9414

AuthUser docs

c77eb33

Signed-off-by: Mihovil Ilakovac <[email protected]>

Comments

3577d36

PR comments

1d52368

PR comments

cd46f61

infomiho force-pushed the update-auth-user-api branch from 1e9969b to cd46f61 Compare April 22, 2024 14:55

Updates the Changelog

6bd82af

infomiho requested a review from Martinsos April 22, 2024 15:31

Martinsos approved these changes Apr 22, 2024

View reviewed changes

infomiho merged commit 0e70547 into main Apr 22, 2024
6 checks passed

infomiho deleted the update-auth-user-api branch April 22, 2024 15:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update AuthUser to offer a better DX #1915

Update AuthUser to offer a better DX #1915

infomiho commented Mar 21, 2024 •

edited

Loading

infomiho Mar 24, 2024

infomiho Mar 24, 2024

infomiho Mar 24, 2024

Martinsos left a comment

Martinsos Mar 26, 2024

Martinsos Mar 26, 2024

infomiho Mar 27, 2024

Martinsos Mar 27, 2024

infomiho Apr 22, 2024

sodic left a comment

sodic Mar 27, 2024

sodic Mar 27, 2024

infomiho Apr 4, 2024

infomiho Apr 11, 2024

sodic Mar 27, 2024

infomiho Apr 4, 2024

Martinsos Apr 4, 2024

sodic Mar 27, 2024

infomiho commented Apr 9, 2024

Martinsos left a comment •

edited

Loading

Martinsos left a comment

		if (user.identities.google !== null) {
		return `Google user ${user.identities.google.id}`

Update AuthUser to offer a better DX #1915

Update AuthUser to offer a better DX #1915

Conversation

infomiho commented Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Martinsos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sodic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

infomiho commented Apr 9, 2024

Martinsos left a comment • edited Loading

Choose a reason for hiding this comment

Martinsos left a comment

Choose a reason for hiding this comment

infomiho commented Mar 21, 2024 •

edited

Loading

Martinsos left a comment •

edited

Loading