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.

Sign up for GitHub

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

Introduce injected session util in user-code #214

Merged
merged 5 commits into from
Nov 21, 2022
Merged

Introduce injected session util in user-code #214

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
537d40e
Introduce injected `session` and `sessionStorage` utils in user-code
jplhomer Nov 18, 2022
fca9488
Revert my accidental await removal
jplhomer Nov 18, 2022
4e55389
Revert this change too
jplhomer Nov 18, 2022
2cec0e5
Nope
jplhomer Nov 18, 2022
356129d
Refactor into a custom `HydrogenSession` class
jplhomer Nov 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions packages/hydrogen-remix/src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export function createRequestHandler(
...options
}: Omit<Parameters<typeof handleRequest>[1], 'loadContext'> &
HydrogenHandlerParams,
customContext?: Record<string, any>,
) => {
try {
if (!cache && !!globalThis.caches) {
Expand All @@ -42,6 +43,7 @@ export function createRequestHandler(
}),
...context,
cache,
...customContext,
},
});

Expand Down
7 changes: 3 additions & 4 deletions templates/demo-store/app/data/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
} from '~/lib/utils';
import invariant from 'tiny-invariant';
import {logout} from '~/routes/account/__private/logout';
import type {HydrogenContext} from '@shopify/hydrogen-remix';
import type {AppLoadContext, HydrogenContext} from '@shopify/hydrogen-remix';
import {type Params} from '@remix-run/react';

export interface CountriesData {
Expand Down Expand Up @@ -932,9 +932,8 @@ export async function getCustomerOrder(
}

export async function getCustomer(
context: HydrogenContext,
context: AppLoadContext & HydrogenContext,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kinda funky: Because we define storefront etc in HydrogenContext from the package, and then augment AppLoadContext in local types, these two sets need to be merged 🤔 Any better way to do this?

{
request,
customerAccessToken,
params,
}: {
Expand All @@ -960,7 +959,7 @@ export async function getCustomer(
* If the customer failed to load, we assume their access token is invalid.
*/
if (!data || !data.customer) {
throw await logout(request, context, params);
throw logout(context, params);
}

return data.customer;
Expand Down
33 changes: 3 additions & 30 deletions templates/demo-store/app/lib/session.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,47 +6,20 @@ import {

let sessionStorage: SessionStorage;

export async function getSession(
request: Request,
context: AppLoadContext & {env?: Record<string, string>},
) {
if (!context.env?.ENCRYPTION_KEY) {
throw new Error('ENCRYPTION_KEY environment variable is not set');
}

export async function getSession(request: Request, secrets: string[]) {
sessionStorage ??= createCookieSessionStorage({
cookie: {
name: 'session',
httpOnly: true,
path: '/',
sameSite: 'lax',
secrets: [context.env.ENCRYPTION_KEY],
secrets,
},
});

const session = await sessionStorage.getSession(
request.headers.get('Cookie'),
);

return {
async get(key: string): Promise<any> {
return await session.get(key);
},

set(key: string, value: any): void {
session.set(key, value);
},

flash(key: string, value: any): void {
session.flash(key, value);
},

unset(key: string): void {
session.unset(key);
},

async commit(): Promise<string> {
return await sessionStorage.commitSession(session);
},
};
return {session, sessionStorage};
}
8 changes: 3 additions & 5 deletions templates/demo-store/app/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {Layout} from '~/components';
import {getLayoutData, getCountries} from '~/data';
import {GenericError} from './components/GenericError';
import {NotFound} from './components/NotFound';
import {getSession} from './lib/session.server';
import {Seo, Debugger} from './lib/seo';

import styles from './styles/app.css';
Expand Down Expand Up @@ -59,9 +58,8 @@ export const meta: MetaFunction = () => ({
viewport: 'width=device-width,initial-scale=1',
});

export async function loader({request, context, params}: LoaderArgs) {
const session = await getSession(request, context);
const cartId = await session.get('cartId');
export async function loader({context, params}: LoaderArgs) {
const cartId = await context.session.get('cartId');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the session property be directly on context, or on context.storefront?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this is unrelated to other storefront things and wouldn't be included (similar to fetch)

Copy link
Contributor

@cartogram cartogram Nov 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree session is unrelated, but maybe a use-case for an API that is like storefront.getCartId(), storefront.getAccessToken().


return defer({
layout: await getLayoutData(context, params),
Expand Down Expand Up @@ -105,7 +103,7 @@ export function CatchBoundary() {
<Links />
</head>
<body>
<Layout layout={root.data.layout}>
<Layout layout={root.data?.layout}>
{isNotFound ? (
<NotFound type={caught.data?.pageType} />
) : (
Expand Down
9 changes: 3 additions & 6 deletions templates/demo-store/app/routes/__resources/cart/LinesAdd.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
} from '@remix-run/react';
import {useIsHydrated} from '~/hooks/useIsHydrated';
import invariant from 'tiny-invariant';
import {getSession} from '~/lib/session.server';
import type {SerializeFrom} from '@remix-run/server-runtime';
import {
type ActionArgs,
Expand Down Expand Up @@ -97,12 +96,10 @@ const ACTION_PATH = '/cart/LinesAdd';
* action that handles cart create (with lines) and lines add
*/
async function action({request, context, params}: ActionArgs) {
const {session, sessionStorage} = context;
const headers = new Headers();

const [session, formData] = await Promise.all([
getSession(request, context),
request.formData(),
]);
const formData = await request.formData();

const rawLines = formData.get('lines')
? (JSON.parse(String(formData.get('lines'))) as LinesAddLine[])
Expand Down Expand Up @@ -133,7 +130,7 @@ async function action({request, context, params}: ActionArgs) {

// cart created - we only need a Set-Cookie header if we're creating
session.set('cartId', cart.id);
headers.set('Set-Cookie', await session.commit());
headers.set('Set-Cookie', await sessionStorage.commitSession(session));
Copy link
Contributor

@juanpprieto juanpprieto Nov 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this a bit confusing that we read from session but we commit via sessionStorage ?

Also the commit feels a little verbose. Could we not have a cleaner commit() that pre-applies the session internally? So we can do something like:

headers.set('Set-Cookie', await sessionStorage.commit());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but it would vary ever so slightly from the official Remix APIs that IMO it would confuse people: https://remix.run/docs/en/v1/api/remix#sessions

👋 @mjackson care to share any context about how this was designed?


return linesAddResponse({
prevCart: null,
Expand Down
4 changes: 1 addition & 3 deletions templates/demo-store/app/routes/account.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,12 @@ import {FeaturedCollections} from '~/components/FeaturedCollections';
import {type LoaderArgs, redirect, json, defer} from '@shopify/hydrogen-remix';
import {flattenConnection} from '@shopify/hydrogen-react';
import {getCustomer} from '~/data';
import {getSession} from '~/lib/session.server';
import {getFeaturedData} from './featured-products';

export async function loader({request, context, params}: LoaderArgs) {
const {pathname} = new URL(request.url);
const session = await getSession(request, context);
const lang = params.lang;
const customerAccessToken = await session.get('customerAccessToken');
const customerAccessToken = await context.session.get('customerAccessToken');
const isAuthenticated = Boolean(customerAccessToken);
const loginPath = lang ? `${lang}/account/login` : '/account/login';

Expand Down
8 changes: 2 additions & 6 deletions templates/demo-store/app/routes/account/__private/address/$id.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
updateCustomerAddress,
updateCustomerDefaultAddress,
} from '~/data';
import {getSession} from '~/lib/session.server';
import {getInputStyleClasses} from '~/lib/utils';
import type {AccountOutletContext} from '../edit';

Expand All @@ -31,12 +30,9 @@ export const handle = {
};

export const action: ActionFunction = async ({request, context, params}) => {
const [formData, session] = await Promise.all([
request.formData(),
getSession(request, context),
]);
const formData = await request.formData();

const customerAccessToken = await session.get('customerAccessToken');
const customerAccessToken = await context.session.get('customerAccessToken');
invariant(customerAccessToken, 'You must be logged in to edit your account.');

const addressId = formData.get('addressId');
Expand Down
8 changes: 2 additions & 6 deletions templates/demo-store/app/routes/account/__private/edit.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import clsx from 'clsx';
import invariant from 'tiny-invariant';
import {Button, Text} from '~/components';
import {getCustomer, updateCustomer} from '~/data';
import {getSession} from '~/lib/session.server';
import {getInputStyleClasses} from '~/lib/utils';

export interface AccountOutletContext {
Expand Down Expand Up @@ -48,12 +47,9 @@ export const handle = {
};

export const action: ActionFunction = async ({request, context, params}) => {
const [formData, session] = await Promise.all([
request.formData(),
getSession(request, context),
]);
const formData = await request.formData();

const customerAccessToken = await session.get('customerAccessToken');
const customerAccessToken = await context.session.get('customerAccessToken');

invariant(
customerAccessToken,
Expand Down
10 changes: 4 additions & 6 deletions templates/demo-store/app/routes/account/__private/logout.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,19 @@ import {
redirect,
} from '@shopify/hydrogen-remix';
import {LoaderArgs} from '@remix-run/server-runtime';
import {getSession} from '~/lib/session.server';

export async function logout(
request: Request,
context: AppLoadContext,
params: LoaderArgs['params'],
) {
const session = await getSession(request, context);
const {session, sessionStorage} = context;
session.unset('customerAccessToken');

return redirect(
params.lang ? `${params.lang}/account/login` : '/account/login',
{
headers: {
'Set-Cookie': await session.commit(),
'Set-Cookie': await sessionStorage.commitSession(session),
},
},
);
Expand All @@ -28,6 +26,6 @@ export async function loader({params}: LoaderArgs) {
return redirect(params.lang ? `${params.lang}/` : '/');
}

export const action: ActionFunction = async ({request, context, params}) => {
return logout(request, context, params);
export const action: ActionFunction = async ({context, params}) => {
return logout(context, params);
};
4 changes: 1 addition & 3 deletions templates/demo-store/app/routes/account/__private/orders.$id.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import type {
} from '@shopify/hydrogen-react/storefront-api-types';
import {Link, Heading, PageHeader, Text} from '~/components';
import {getCustomerOrder} from '~/data';
import {getSession} from '~/lib/session.server';

export const meta: MetaFunction = ({data}) => ({
title: `Order ${data?.order?.name}`,
Expand All @@ -32,8 +31,7 @@ export async function loader({request, context, params}: LoaderArgs) {

invariant(orderToken, 'Order token is required');

const session = await getSession(request, context);
const customerAccessToken = await session.get('customerAccessToken');
const customerAccessToken = await context.session.get('customerAccessToken');

if (!customerAccessToken) {
return redirect(
Expand Down
10 changes: 4 additions & 6 deletions templates/demo-store/app/routes/account/__public/activate.$id.$activationToken.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
import {Form, useActionData} from '@remix-run/react';
import {useRef, useState} from 'react';
import {activateAccount} from '~/data';
import {getSession} from '~/lib/session.server';
import {getInputStyleClasses} from '~/lib/utils';

type ActionData = {
Expand Down Expand Up @@ -37,10 +36,7 @@ export const action: ActionFunction = async ({
});
}

const [formData, session] = await Promise.all([
request.formData(),
getSession(request, context),
]);
const formData = await request.formData();

const password = formData.get('password');
const passwordConfirm = formData.get('passwordConfirm');
Expand All @@ -64,11 +60,13 @@ export const action: ActionFunction = async ({
password,
});

const {session, sessionStorage} = context;

session.set('customerAccessToken', accessToken);

return redirect(lang ? `${lang}/account` : '/account', {
headers: {
'Set-Cookie': await session.commit(),
'Set-Cookie': await sessionStorage.commitSession(session),
},
});
} catch (error: any) {
Expand Down
14 changes: 5 additions & 9 deletions templates/demo-store/app/routes/account/__public/login.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,15 @@ import {
import {Form, useActionData, useLoaderData} from '@remix-run/react';
import {useState} from 'react';
import {login} from '~/data';
import {getSession} from '~/lib/session.server';
import {getInputStyleClasses} from '~/lib/utils';
import {Link} from '~/components';

export const handle = {
isPublic: true,
};

export async function loader({request, context, params}: LoaderArgs) {
const session = await getSession(request, context);
const customerAccessToken = await session.get('customerAccessToken');
export async function loader({context, params}: LoaderArgs) {
const customerAccessToken = await context.session.get('customerAccessToken');

if (customerAccessToken) {
return redirect(params.lang ? `${params.lang}/account` : '/account');
Expand All @@ -36,10 +34,7 @@ type ActionData = {
const badRequest = (data: ActionData) => json(data, {status: 400});

export const action: ActionFunction = async ({request, context, params}) => {
const [formData, session] = await Promise.all([
request.formData(),
getSession(request, context),
]);
const formData = await request.formData();

const email = formData.get('email');
const password = formData.get('password');
Expand All @@ -57,11 +52,12 @@ export const action: ActionFunction = async ({request, context, params}) => {

try {
const customerAccessToken = await login(context, {email, password});
const {session, sessionStorage} = context;
session.set('customerAccessToken', customerAccessToken);

return redirect(params.lang ? `${params.lang}/account` : '/account', {
headers: {
'Set-Cookie': await session.commit(),
'Set-Cookie': await sessionStorage.commitSession(session),
},
});
} catch (error: any) {
Expand Down
6 changes: 2 additions & 4 deletions templates/demo-store/app/routes/account/__public/recover.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ import {
import {Form, useActionData} from '@remix-run/react';
import {useState} from 'react';
import {sendPasswordResetEmail} from '~/data';
import {getSession} from '~/lib/session.server';
import {Link} from '~/components';
import {getInputStyleClasses} from '~/lib/utils';

export async function loader({request, context, params}: LoaderArgs) {
const session = await getSession(request, context);
const customerAccessToken = await session.get('customerAccessToken');
export async function loader({context, params}: LoaderArgs) {
const customerAccessToken = await context.session.get('customerAccessToken');

if (customerAccessToken) {
return redirect(params.lang ? `${params.lang}/account` : '/account');
Expand Down
Loading