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

Add sessions #508

Merged
merged 25 commits into from
Sep 4, 2022
Merged

Add sessions #508

merged 25 commits into from
Sep 4, 2022

Conversation

AlecAivazis
Copy link
Collaborator

@AlecAivazis AlecAivazis commented Sep 1, 2022

This is a continuation of @fehnomenal's work in #489 that started the support for user-based sessions (ie, authentication). There are 2 parts to this feature:

  1. Session data is set in hooks.js using the houdini client's new setSession(event, { ... }) method
  2. Session data is passed to the client's fetch function (similar to how it was previously done)

To help everyone out, please make sure your PR does the following:

  • If applicable, add a test that would fail without this fix
  • Make sure the unit and integration tests pass locally with pnpm run tests and cd integration && pnpm run tests
  • Includes a changeset if your fix affects the user with pnpm changeset

What's left

  • Get confirmation from fehnomenal that I didn't miss anything in the transfer
  • integration tests for generated layout files (with typecheck for session)
  • add warnings (look for todo)
  • document no more client.init
  • update init command to not have client.init
  • update authentication guide
  • use App.Session type
  • onError client side navgiation integration tests should fail
  • passing session to mutations

* Allow passing session data to the fetch function

* Prefer the client side session

* Receive the server session only in the browser

* Separate client and server sessions more clearly

* make test more resilient

* fake root layout file

* fake +layout.server.js

* kit transform threads session from +layout.server.js to client

* remove client.init

Co-authored-by: Andreas Fehn <[email protected]>
@vercel
Copy link

vercel bot commented Sep 1, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
docs-next ✅ Ready (Inspect) Visit Preview Sep 4, 2022 at 7:50PM (UTC)

@changeset-bot
Copy link

changeset-bot bot commented Sep 1, 2022

🦋 Changeset detected

Latest commit: a952aeb

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@AlecAivazis AlecAivazis changed the title Add sessions (#507) Add sessions Sep 1, 2022
@endigma
Copy link
Contributor

endigma commented Sep 2, 2022

does this need to use hooks? I think the page data is available to all children? Rich's post on the "removing sessions" discussion mentions doing like:

// src/routes/+layout.server.ts

import type { LayoutServerLoad } from "./$types"

export const load: LayoutServerLoad = async function ({ request }) {
	return {
		token: request.headers.get("Authorization"),
	}
}

am I missing something?

@jycouet
Copy link
Contributor

jycouet commented Sep 2, 2022

does this need to use hooks? I think the page data is available to all children? Rich's post on the "removing sessions" discussion mentions doing like:

// src/routes/+layout.server.ts

import type { LayoutServerLoad } from "./$types"

export const load: LayoutServerLoad = async function ({ request }) {
	return {
		token: request.headers.get("Authorization"),
	}
}

am I missing something?

You are right, you can get the token like this.

But then, you have to pass it all the time "manually" to do graphql operations.
Like:

// +page.ts _pseudo code_

export const load: PageLoad = async (event) => {
  const { token } = await event.parent();
  GQL_GetUser.fetch({ variables: xxx, metadata: { token } })
}
// client.ts _pseudo code_

import type { RequestHandlerArgs } from '$houdini'
import { HoudiniClient } from '$houdini'

async function fetchQuery({ fetch, text = '', variables = {}, metadata}: RequestHandlerArgs) {
    const url = 'http://.../graphql'

    const result = await fetch(url, {
        method: 'POST',
        headers: {
            'Content-Type': 'application/json',
            Authorization: `Bearer ${metadata?.token}`
        },
        body: JSON.stringify({
            query: text,
            variables
        })
    })
    return await result.json()
}

export default new HoudiniClient(fetchQuery)

You have to do it like this because we can't know that you will name your token. token? superToken? Maybe you will have a lot of other things in data that are not for Houdini... you will have to await parent...

That's a lot of constraints. But it's possible 👍


With hooks and session approach, token authentication is done globally.
And usage becomes lighter focusing on variables & business logic in components.

// +page.ts _pseudo code_

export const load: PageLoad = async (event) => {
  GQL_GetUser.fetch({ variables: xxx })
}

It improves the DX a lot I think 👍
And it's enabling Houdini to generate a lot of boilerplate for you (automatic loading, component queries, ...)

@endigma
Copy link
Contributor

endigma commented Sep 2, 2022

Theoretically you could have it so the user passes a function that takes load args and produces a token, but I do think your method is sound.

On another topic, I'm concerned about how to set this manually, for logins and logouts. Would this function off cookies or something more advanced?

@jycouet
Copy link
Contributor

jycouet commented Sep 2, 2022

I would like to provide few complete guides later on these common topics 👌
Follow our work 👍

@david-plugge
Copy link
Contributor

Nice work!

@AlecAivazis
Copy link
Collaborator Author

I'm going to merge this so I can deploy a new version and finally start migrating my applications to the new svelte kit and houdini 🎉 Thanks again everyone for all the help!

@AlecAivazis AlecAivazis merged commit 60ecb33 into main Sep 4, 2022
@AlecAivazis AlecAivazis deleted the session branch September 4, 2022 21:07
@endigma
Copy link
Contributor

endigma commented Sep 4, 2022

Docs coming soon?

@jycouet
Copy link
Contributor

jycouet commented Sep 4, 2022

Actually, all PR has their own docs (check on top "the Vercel preview")

In general .next version is available here: https://docs-next-kohl.vercel.app/guides/authentication

@github-actions github-actions bot mentioned this pull request Sep 9, 2022
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.

4 participants