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

Minor template updates and fixes #1341

Merged
merged 6 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .changeset/weak-windows-burn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'demo-store': patch
'@shopify/create-hydrogen': patch
---

Improved types of `HydrogenSession` when accessing `session.get('customerAccessToken')`.
3 changes: 2 additions & 1 deletion examples/customer-api/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
"isolatedModules": true,
"esModuleInterop": true,
"jsx": "react-jsx",
"moduleResolution": "node",
"moduleResolution": "Bundler",
"resolveJsonModule": true,
"module": "ES2022",
"target": "ES2022",
"strict": true,
"allowJs": true,
Expand Down
6 changes: 2 additions & 4 deletions package-lock.json

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

38 changes: 19 additions & 19 deletions templates/demo-store/app/lib/session.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import {
* swap out the cookie-based implementation with something else!
*/
export class HydrogenSession {
constructor(
private sessionStorage: SessionStorage,
private session: Session,
) {
this.session = session;
this.sessionStorage = sessionStorage;
#sessionStorage;
#session;

constructor(sessionStorage: SessionStorage, session: Session) {
this.#sessionStorage = sessionStorage;
this.#session = session;
Comment on lines -13 to +18
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 was working fine but TS parameter properties is a bit obscure. I think using assignments is easier to understand.
Plus, this can now rely on the private members of native JS (this.#...).

}

static async init(request: Request, secrets: string[]) {
Expand All @@ -34,31 +34,31 @@ export class HydrogenSession {
return new this(storage, session);
}

has(key: string) {
return this.session.has(key);
get has() {
return this.#session.has;
}
Comment on lines -37 to 39
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By turning method wrappers into getters like this, we delegate the type to the actual session without losing information due to key: string (i.e. string is more generic that what the session expects).

Open to ideas for a simpler syntax that keeps the types right.


get(key: string) {
return this.session.get(key);
get get() {
return this.#session.get;
}

destroy() {
return this.sessionStorage.destroySession(this.session);
get flash() {
return this.#session.flash;
}

flash(key: string, value: any) {
this.session.flash(key, value);
get unset() {
return this.#session.unset;
}

unset(key: string) {
this.session.unset(key);
get set() {
return this.#session.set;
}

set(key: string, value: any) {
this.session.set(key, value);
destroy() {
return this.#sessionStorage.destroySession(this.#session);
}

commit() {
return this.sessionStorage.commitSession(this.session);
return this.#sessionStorage.commitSession(this.#session);
}
}
11 changes: 0 additions & 11 deletions templates/demo-store/app/lib/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {useLocation, useMatches} from '@remix-run/react';
import {parse as parseCookie} from 'worktop/cookie';
import type {MoneyV2} from '@shopify/hydrogen/storefront-api-types';
import typographicBase from 'typographic-base';

Expand Down Expand Up @@ -332,13 +331,3 @@ export function isLocalPath(url: string) {

return false;
}

/**
* Shopify's 'Online Store' stores cart IDs in a 'cart' cookie.
* By doing the same, merchants can switch from the Online Store to Hydrogen
* without customers losing carts.
*/
export function getCartId(request: Request) {
const cookies = parseCookie(request.headers.get('Cookie') || '');
return cookies.cart ? `gid://shopify/Cart/${cookies.cart}` : undefined;
}
2 changes: 1 addition & 1 deletion templates/demo-store/app/routes/($locale).account.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export async function loader({request, context, params}: LoaderArgs) {
const {pathname} = new URL(request.url);
const locale = params.locale;
const customerAccessToken = await context.session.get('customerAccessToken');
const isAuthenticated = Boolean(customerAccessToken);
const isAuthenticated = !!customerAccessToken;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Boolean constructor here does not inform TS that customerAccessToken is defined or not after asserting isAuthenticated. !! does the trick (TIL).

const loginPath = locale ? `/${locale}/account/login` : '/account/login';
const isAccountPage = /^\/account\/?$/.test(pathname);

Expand Down
3 changes: 1 addition & 2 deletions templates/demo-store/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@
"react-use": "^17.4.0",
"schema-dts": "^1.1.0",
"tiny-invariant": "^1.2.0",
"typographic-base": "^1.0.4",
"worktop": "^0.7.3"
"typographic-base": "^1.0.4"
},
"devDependencies": {
"@remix-run/dev": "1.19.1",
Expand Down
13 changes: 10 additions & 3 deletions templates/demo-store/remix.env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,24 @@ declare global {
}
}

/**
* Declare local additions to `AppLoadContext` to include the session utilities we injected in `server.ts`.
*/
declare module '@shopify/remix-oxygen' {
/**
* Declare local additions to the Remix loader context.
*/
export interface AppLoadContext {
waitUntil: ExecutionContext['waitUntil'];
session: HydrogenSession;
storefront: Storefront;
cart: HydrogenCart;
env: Env;
}

/**
* Declare the data we expect to access via `context.session`.
*/
export interface SessionData {
customerAccessToken: string;
}
}

// Needed to make this file a module.
Expand Down
2 changes: 1 addition & 1 deletion templates/demo-store/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default {
throw new Error('SESSION_SECRET environment variable is not set');
}

const waitUntil = (p: Promise<any>) => executionContext.waitUntil(p);
const waitUntil = executionContext.waitUntil.bind(executionContext);
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 change makes it easier to infer the type when using JSDocs because we are not wrapping waitUntil anymore

const [cache, session] = await Promise.all([
caches.open('hydrogen'),
HydrogenSession.init(request, [env.SESSION_SECRET]),
Expand Down
2 changes: 1 addition & 1 deletion templates/demo-store/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"isolatedModules": true,
"esModuleInterop": true,
"jsx": "react-jsx",
"moduleResolution": "node",
"moduleResolution": "Bundler",
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 new resolution mode is supposed to be appropriate for ESBuild/etc.

"resolveJsonModule": true,
"module": "ES2022",
"target": "ES2022",
Expand Down
2 changes: 1 addition & 1 deletion templates/hello-world/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"isolatedModules": true,
"esModuleInterop": true,
"jsx": "react-jsx",
"moduleResolution": "node",
"moduleResolution": "Bundler",
"resolveJsonModule": true,
"module": "ES2022",
"target": "ES2022",
Expand Down
10 changes: 6 additions & 4 deletions templates/skeleton/app/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ export async function loader({context}: LoaderArgs) {

// validate the customer access token is valid
const {isLoggedIn, headers} = await validateCustomerAccessToken(
customerAccessToken,
session,
customerAccessToken,
);

// defer the cart query by not awaiting it
Expand Down Expand Up @@ -198,17 +198,19 @@ export function CatchBoundary() {
* ```
* */
async function validateCustomerAccessToken(
customerAccessToken: CustomerAccessToken,
session: HydrogenSession,
customerAccessToken?: CustomerAccessToken,
) {
let isLoggedIn = false;
const headers = new Headers();
if (!customerAccessToken?.accessToken || !customerAccessToken?.expiresAt) {
return {isLoggedIn, headers};
}
const expiresAt = new Date(customerAccessToken.expiresAt);
const dateNow = new Date();

const expiresAt = new Date(customerAccessToken.expiresAt).getTime();
const dateNow = Date.now();
const customerAccessTokenExpired = expiresAt < dateNow;

if (customerAccessTokenExpired) {
session.unset('customerAccessToken');
headers.append('Set-Cookie', await session.commit());
Expand Down
2 changes: 1 addition & 1 deletion templates/skeleton/app/routes/account.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export async function loader({request, context}: LoaderArgs) {
const {session, storefront} = context;
const {pathname} = new URL(request.url);
const customerAccessToken = await session.get('customerAccessToken');
const isLoggedIn = Boolean(customerAccessToken?.accessToken);
const isLoggedIn = !!customerAccessToken?.accessToken;
const isAccountHome = pathname === '/account' || pathname === '/account/';
const isPrivateRoute =
/^\/account\/(orders|orders\/.*|profile|addresses|addresses\/.*)$/.test(
Expand Down
2 changes: 1 addition & 1 deletion templates/skeleton/app/routes/cart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export async function action({request, context}: ActionArgs) {
case CartForm.ACTIONS.BuyerIdentityUpdate: {
result = await cart.updateBuyerIdentity({
...inputs.buyerIdentity,
customerAccessToken,
customerAccessToken: customerAccessToken?.accessToken,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this was an actual bug now caught by the enhanced types.

});
break;
}
Expand Down
14 changes: 11 additions & 3 deletions templates/skeleton/remix.env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import '@total-typescript/ts-reset';

import type {Storefront, HydrogenCart} from '@shopify/hydrogen';
import type {CustomerAccessToken} from '@shopify/hydrogen/storefront-api-types';
import type {HydrogenSession} from './server';

declare global {
Expand All @@ -26,15 +27,22 @@ declare global {
}
}

/**
* Declare local additions to `AppLoadContext` to include the session utilities we injected in `server.ts`.
*/
declare module '@shopify/remix-oxygen' {
/**
* Declare local additions to the Remix loader context.
*/
export interface AppLoadContext {
env: Env;
cart: HydrogenCart;
storefront: Storefront;
session: HydrogenSession;
waitUntil: ExecutionContext['waitUntil'];
}

/**
* Declare the data we expect to access via `context.session`.
*/
export interface SessionData {
customerAccessToken: CustomerAccessToken;
}
}
39 changes: 21 additions & 18 deletions templates/skeleton/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default {
throw new Error('SESSION_SECRET environment variable is not set');
}

const waitUntil = (p: Promise<any>) => executionContext.waitUntil(p);
const waitUntil = executionContext.waitUntil.bind(executionContext);
const [cache, session] = await Promise.all([
caches.open('hydrogen'),
HydrogenSession.init(request, [env.SESSION_SECRET]),
Expand Down Expand Up @@ -99,10 +99,13 @@ export default {
* swap out the cookie-based implementation with something else!
*/
export class HydrogenSession {
constructor(
private sessionStorage: SessionStorage,
private session: Session,
) {}
#sessionStorage;
#session;

constructor(sessionStorage: SessionStorage, session: Session) {
this.#sessionStorage = sessionStorage;
this.#session = session;
}

static async init(request: Request, secrets: string[]) {
const storage = createCookieSessionStorage({
Expand All @@ -120,32 +123,32 @@ export class HydrogenSession {
return new this(storage, session);
}

has(key: string) {
return this.session.has(key);
get has() {
return this.#session.has;
}

get(key: string) {
return this.session.get(key);
get get() {
return this.#session.get;
}

destroy() {
return this.sessionStorage.destroySession(this.session);
get flash() {
return this.#session.flash;
}

flash(key: string, value: any) {
this.session.flash(key, value);
get unset() {
return this.#session.unset;
}

unset(key: string) {
this.session.unset(key);
get set() {
return this.#session.set;
}

set(key: string, value: any) {
this.session.set(key, value);
destroy() {
return this.#sessionStorage.destroySession(this.#session);
}

commit() {
return this.sessionStorage.commitSession(this.session);
return this.#sessionStorage.commitSession(this.#session);
}
}

Expand Down
2 changes: 1 addition & 1 deletion templates/skeleton/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"isolatedModules": true,
"esModuleInterop": true,
"jsx": "react-jsx",
"moduleResolution": "node",
"moduleResolution": "Bundler",
"resolveJsonModule": true,
"module": "ES2022",
"target": "ES2022",
Expand Down