Create `createHydrogenContext` that combine `createStorefrontClient`,`createCustomerAccountClient` and `createCartHandler` #2333

michenly · 2024-07-15T20:49:22Z

Create HydrogenContext so individual env.d.ts dont need to define it themselves anymore
Create createHydrogenContext that combine options from storefront, customerAccount and cart. This is to prepared for an combined context object as well.
Ensure both createHydrogenContext and cartGetIdDefault are compatible with both Request and CrossRuntimeRequest. Thus compatible with express.

⚠️ Noted in all the examples below that createHydrogenContext does type inference and you never need to pass in generic. However, in global type file env.d.ts, generics are required since there is nothing to infer from.

Default usage

const hydrogenContext = createHydrogenContext({
        env,
        request,
        cache,
        waitUntil,
        session,
        i18n: {language: 'EN', country: 'US'},
        cart: {
          queryFragment: CART_QUERY_FRAGMENT,
        },
});

const handleRequest = createRequestHandler({
        build: remixBuild,
        mode: process.env.NODE_ENV,
        getLoadContext: (): AppLoadContext => ({
          ...hydrogenContext,
          // declare additional Remix loader context here
        }),
    });

Usage in env.d.ts

interface AppLoadContext extends HydrogenContext<AppSession> {
    // declare additional Remix loader context here
}

With customized cart methods

// server.ts
const hydrogenContext = createHydrogenContext({
        env,
        request,
        cache,
        waitUntil,
        session,
        i18n: {language: 'EN', country: 'US'},
        cart: {
          queryFragment: CART_QUERY_FRAGMENT,
          customMethods: {
            updateLineByOptions: async (
              productId: string,
              selectedOptions: SelectedOptionInput[],
              line: CartLineUpdateInput,
            ) => {
              const {product} = await hydrogenContext.storefront.query(
                PRODUCT_VARIANT_QUERY,
                {
                  variables: {
                    productId,
                    selectedOptions,
                  },
                },
              );

              const lines = [
                {...line, merchandiseId: product?.selectedVariant?.id},
              ];

              return await cartLinesUpdateDefault({
                storefront: hydrogenContext.storefront,
                getCartId: cartGetIdDefault(request.headers),
              })(lines);
            },
          },
        },
      });

// in env.d.ts

interface AppLoadContext
    extends HydrogenContext<
      AppSession,
      {
        updateLineByOptions: (
          productId: string,
          selectedOptions: SelectedOptionInput[],
          line: CartLineUpdateInput,
        ) => Promise<CartQueryDataReturn>;
      }
    > {
    // declare additional Remix loader context here
  }

shopify · 2024-07-15T20:49:39Z

Oxygen deployed a preview of your mc-create-shopify branch. Details:

Storefront	Status	Preview link	Deployment details	Last update (UTC)
metaobjects	✅ Successful (Logs)	Preview deployment	Inspect deployment	August 2, 2024 3:29 PM
custom-cart-method	✅ Successful (Logs)	Preview deployment	Inspect deployment	August 2, 2024 3:29 PM
classic-remix	✅ Successful (Logs)	Preview deployment	Inspect deployment	August 2, 2024 3:29 PM
optimistic-cart-ui	✅ Successful (Logs)	Preview deployment	Inspect deployment	August 2, 2024 3:29 PM
third-party-queries-caching	✅ Successful (Logs)	Preview deployment	Inspect deployment	August 2, 2024 3:29 PM
Skeleton (skeleton.hydrogen.shop)	✅ Successful (Logs)	Preview deployment	Inspect deployment	August 2, 2024 9:43 PM

Learn more about Hydrogen's GitHub integration.

frandiox

Great job Michelle, looking much simpler 🔥

Just a partial review for now with quick things I saw.

packages/cli/src/lib/setups/i18n/replacers.test.ts

frandiox · 2024-07-18T01:11:38Z

packages/hydrogen/src/getStorefrontHeaders.ts

+export function getStorefrontHeaders(
+  request: Request | CrossRuntimeRequest,
+): StorefrontHeaders {
+  const headers = request.headers;
+  return {
+    requestGroupId: (headers.get ? headers.get('request-id') : null) || null,
+    buyerIp: (headers.get ? headers.get('oxygen-buyer-ip') : null) || null,
+    cookie: (headers.get ? headers.get('cookie') : null) || null,
+    purpose: (headers.get ? headers.get('purpose') : null) || null,
+  };
+}


The idea of having getStorefrontHeaders in remix-oxygen package is because it's unique to the Oxygen platform. In Node or somewhere else, you'd get buyerIp, cookie, etc. in other ways.

Therefore, if we want to move this function here, at the very least I think we should make it obvious this only works on Oxygen or it will be misleading. Perhaps getStorefrontFromOxygenHeaders?

--

In fact, I'm not even sure about this because within Hydrogen we don't know if we're running on Oxygen or not. So it's the user the one who should be on charge of passing buyerIp etc. 🤔

E.g. say we are on Express and we try internally to access 'oxygen-buyer-ip' but that simply returns null. We should let the user know we couldn't find the IP address. We used to do this by forcing the user to pass this parameter manually. I guess we could just warn/throw now... not sure :/

The main reason why I was moving getStorefrontHeaders into @Shopify/hydrogen was because I can't import @shopify/remix-oxygen. It created circular import and won't build.

Maybe what I can do is just a local method that account for both without exporting it at all?

Looking through the storefront code, looks like we generate a UUID for requestGroupId, buyerIp and cookie gets default to empty string, and purpose us just left as null as default.

I dont think we should throw since we currently dont, maybe warn? Not sure if we really need to thou.

Looking through the storefront code, looks like we generate a UUID for requestGroupId, buyerIp and cookie gets default to empty string, and purpose us just left as null as default.
I dont think we should throw since we currently dont, maybe warn? Not sure if we really need to thou.

True, I thought these params were required in TS at least, but they are actually optional. In that case we probably don't need to throw/warn, although we should probably consider warning eventually...

That said, it would still be better to use the getHeader utility so that we know how to get headers from Node requests (those that are present, like purpose).

frandiox · 2024-07-18T01:17:23Z

packages/hydrogen/src/cart/cartGetIdDefault.ts

+  requestHeaders: Headers | CrossRuntimeRequest['headers'],
+) => {
+  const cookies = parse(
+    (requestHeaders.get ? requestHeaders.get('Cookie') : undefined) || '',


You can use getHeader from utils/request.ts, it understands CrossRuntimeRequest.headers already.
We might need to change the type here to be requestHeaders: CrossRuntimeRequest['headers'] though 🤔

We might need to change the type here to be requestHeaders: CrossRuntimeRequest['headers'] though 🤔

You mean instead of both types of requestHeaders: Headers | CrossRuntimeRequest['headers'], just account for one? I thought the point is we will try to cover both shape of headers.

The idea is that CrossRuntimeRequest['headers'] already contains both types. It basically requests only the minimum type that work in both runtimes. Am I wrong? 🤔

I change the type to use CrossRuntimeRequest['headers'] only. It looks like it does contain both so we are good!

However, I dont think I can use the getHeader function here since whats being pass in is the header and getHeader uses the request. (If I change that it will be breaking change)

I end up creating getHeaderValue for this purpose.

I think you would just need to pass is in an object, like getHeader({headers}, key), because the rest of the keys in CrossRuntimeRequest are optional. That said, getHeaderValue is OK as well.

templates/skeleton/server.ts

templates/skeleton/env.d.ts

packages/hydrogen/src/createHydrogenContext.ts

michenly · 2024-07-30T20:43:56Z

/snapit

github-actions · 2024-07-30T20:46:17Z

🫰✨ Thanks @michenly! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

"@shopify/cli-hydrogen": "0.0.0-snapshot-20240730204555",
"@shopify/create-hydrogen": "0.0.0-snapshot-20240730204555",
"@shopify/hydrogen": "0.0.0-snapshot-20240730204555"

michenly · 2024-07-30T20:50:04Z

@frandiox I managed to create a JS project using the current template with CustomAppLoadContext

Not exactly sure how to verify this, but here it the output

frandiox · 2024-07-31T12:38:33Z

I think that @typedef {Class<createAppLoadContext>>} is not going to work. The way to test this is basically just checking for variable autocompletion when you do context. in a loader. You should get all the types there even in JS files, because the jsconfig.json is using d.ts files.

The problem is that the typedef is not correct, so AppLoadContext is not going to get correct types.

--

We can either fix the morph code to infer the typedef correctly for this case... or maybe change env.d.ts to import type {createAppLoadContext} from '...'; --- interface AppLoadContext extends (Awaited<ReturnType<typeof createAppLoadContext>>) {} (basically we just move the tricky type to the d.ts so that we don't need to generate JSDoc for it).

michenly · 2024-07-31T16:29:13Z

@frandiox I went with your second suggestion as this will be a much easier maintenance going forward. Thank you for the suggestion!

michenly · 2024-07-31T22:30:38Z

@frandiox @blittle
The replacer method had been updated and I 🎩 with JS strategy, and all three i18n strategies as well.
Please give the PR a review 🙏

blittle

Great work Michelle. Just a few minor things.

blittle · 2024-08-02T14:06:19Z

.changeset/light-boxes-smile.md

+'@shopify/hydrogen': patch
+---
+
+Create `createHydrogenContext` that combined `createStorefrontClient`, `createCustomerAccountClient` and `createCartHandler`.


We can fix right before release, but in my opinion all of the above changeset information about setting it up should be here. The feature is a new feature to hydrogen, so we should be highlighting how to adopt and use it here. We need to trigger a new @shopify/create-hydrogen release, but beyond that, I think it's confusing spread release notes between the two. But maybe we should align with @juanpprieto and @frandiox.

I think the long description example should be in the skeleton changeset, no? That's where we put changes to the skeleton so that devs with existing projects can update their code.

This one is good for hydrogen, and create-hydrogen can just have a small description as well, or just "starter template updated"

I can see the point for both.

I think it makes a lot of sense to keep the changset minimal and relevant, written in the tone for the package at hand. But for a new feature we want the folks migrate to, we do need a way to highlight it as well.

To combined the two opinions, how about we keep the changeset minimal. But use the upgrade steps and the blog post to hight light new features.

.changeset/healthy-rats-confess.md

packages/cli/assets/i18n/mock-i18n-types.ts

packages/cli/src/lib/setups/i18n/replacers.ts

packages/hydrogen/src/createHydrogenContext.doc.ts

packages/hydrogen/src/createHydrogenContext.ts

blittle · 2024-08-02T14:34:11Z

packages/hydrogen/src/createHydrogenContext.ts

+    getCartId: cartOptions.getId || cartGetIdDefault(request.headers),
+    setCartId: cartOptions.setId || cartSetIdDefault(),
+    cartQueryFragment: cartOptions.queryFragment,
+    cartMutateFragment: cartOptions.mutateFragment,


🥳 the default getCartId and setCartId defaults. Should we also have a default for cartQueryFragment?

cartQueryFragment wont be included in code gen type if the defaults move to @shopify/hydrogen
it was left in the user code for this reason.

That seems unfortunate, I wonder if we can fix codegen to do that? But probably a different PR.

@frandiox wondering if you know this is a simple thing to fix or not?

Co-authored-by: Bret Little <[email protected]>

frandiox

I think it's looking good 🎉 Minor comments below:

frandiox · 2024-08-02T15:52:30Z

.changeset/light-boxes-smile.md

+'@shopify/hydrogen': patch
+---
+
+Create `createHydrogenContext` that combined `createStorefrontClient`, `createCustomerAccountClient` and `createCartHandler`.


I think the long description example should be in the skeleton changeset, no? That's where we put changes to the skeleton so that devs with existing projects can update their code.

This one is good for hydrogen, and create-hydrogen can just have a small description as well, or just "starter template updated"

packages/cli/src/lib/setups/i18n/replacers.ts

packages/cli/assets/i18n/subdomains.ts

packages/cli/src/lib/setups/i18n/replacers.ts

frandiox · 2024-08-02T16:31:49Z

packages/cli/src/lib/setups/i18n/replacers.ts

+    // update imported type from mock-i18n-types to @shopify/hydrogen with TS file
+    const importFromNode = root.find({


This is such a simple replacement that I think it's better to simply use RegExp instead of Ast-Grep: content.replace(/'./mock-api-types'/, '@shopify/hydrogen')`

frandiox · 2024-08-02T16:36:14Z

packages/hydrogen/src/createHydrogenContext.example.jsx

+class AppSession {
+  isPending = false;


Could we just show a permalink to the session implementation file on GitHub? 🙈 (it's not the main thing being showcased here).

We actually do this in a few other places too.

Without the local implementation of AppSession, the type wont pass.
But it was noted to be a smelly since it does indicate a non-complete example.

I can use @ts-ignore and just leave the example in-complete. Let me know how this feel, if folks are ok with it I will make this change in all the examples.

//@ts-ignore See example implementation in https://github.com/Shopify/hydrogen/blob/main/templates/skeleton/app/lib/session.ts import {AppSession} from '~/lib/session';

frandiox · 2024-08-02T16:39:17Z

packages/hydrogen/src/createHydrogenContext.ts

+export interface HydrogenContext<
+  TSession extends HydrogenSession = HydrogenSession,
+  TCustomMethods extends CustomMethodsBase | undefined = undefined,
+  TI18n extends I18nBase = I18nBase,
+  TEnv extends HydrogenEnv = Env,
+> {
+  /** A GraphQL client for querying the [Storefront API](https://shopify.dev/docs/api/storefront). */
+  storefront: StorefrontClient<TI18n>['storefront'];


Now that we have this context, we could bubble up some properties in storefront like cache and i18n. But we can do this later.

as in return cache and i18n in context as well?

Yeah, but we can think more about this I guess.

@blittle what do you think?
I can add cache fairly quickly but I dont really have a pulse on when we will need to reach for it outside of storefrontClient

i18n would be great to have in context, but would like to do that in a separate PR as it require more testing to ensure changing locale works properly with demo store.

packages/hydrogen/src/createHydrogenContext.ts

templates/skeleton/server.ts

frandiox · 2024-08-02T17:03:10Z

packages/hydrogen/src/cart/cartGetIdDefault.ts

+  requestHeaders: Headers | CrossRuntimeRequest['headers'],
+) => {
+  const cookies = parse(
+    (requestHeaders.get ? requestHeaders.get('Cookie') : undefined) || '',


I think you would just need to pass is in an object, like getHeader({headers}, key), because the rest of the keys in CrossRuntimeRequest are optional. That said, getHeaderValue is OK as well.

This comment has been minimized.

Sign in to view

michenly force-pushed the mc-combined-context branch 2 times, most recently from c163a20 to f3d5790 Compare July 15, 2024 20:55

michenly force-pushed the mc-create-shopify branch from 851271b to 16598b1 Compare July 15, 2024 20:57

michenly self-assigned this Jul 15, 2024

michenly requested a review from a team July 15, 2024 21:44

michenly force-pushed the mc-create-shopify branch from fe5cfc9 to d8d45cb Compare July 15, 2024 22:49

michenly changed the title ~~Create createShopifyHandler that combine options from storefront, customerAccount and cart~~ Create createShopifyHandler that combine options from createStorefrontClient,createCustomerAccountClient and createCartHandler Jul 15, 2024

michenly changed the title ~~Create createShopifyHandler that combine options from createStorefrontClient,createCustomerAccountClient and createCartHandler~~ Create createShopifyHandler that combine createStorefrontClient,createCustomerAccountClient and createCartHandler Jul 16, 2024

michenly force-pushed the mc-create-shopify branch 5 times, most recently from b75bc76 to 1d9ed1c Compare July 16, 2024 23:35

michenly mentioned this pull request Jul 17, 2024

Create ShopifyEnv and use it for all the env.d.ts #2332

Closed

michenly changed the base branch from mc-combined-context to main July 17, 2024 16:13

michenly force-pushed the mc-create-shopify branch from b8c6f0e to 0cb50f9 Compare July 17, 2024 19:56

frandiox reviewed Jul 18, 2024

View reviewed changes

michenly commented Jul 18, 2024

View reviewed changes

templates/skeleton/server.ts Outdated Show resolved Hide resolved

michenly commented Jul 18, 2024

View reviewed changes

templates/skeleton/env.d.ts Outdated Show resolved Hide resolved

michenly commented Jul 18, 2024

View reviewed changes

templates/skeleton/env.d.ts Outdated Show resolved Hide resolved

michenly commented Jul 18, 2024

View reviewed changes

packages/hydrogen/src/createHydrogenContext.ts Outdated Show resolved Hide resolved

michenly changed the title ~~Create createShopifyHandler that combine createStorefrontClient,createCustomerAccountClient and createCartHandler~~ Create createHydrogenContext that combine createStorefrontClient,createCustomerAccountClient and createCartHandler Jul 18, 2024

michenly force-pushed the mc-create-shopify branch 6 times, most recently from f868146 to 0c722ae Compare July 22, 2024 18:24

fix header according to comment

e362948

Shopify deleted a comment from github-actions bot Jul 30, 2024

michenly force-pushed the mc-create-shopify branch from c4d25ce to e362948 Compare July 30, 2024 20:55

move type definition from context to env.d.ts

33df563

michenly force-pushed the mc-create-shopify branch 3 times, most recently from e673a5d to b9a66e2 Compare July 31, 2024 22:02

modify i18n replacer method to modify the new context file

8536f08

michenly force-pushed the mc-create-shopify branch from b9a66e2 to 8536f08 Compare July 31, 2024 22:29

blittle approved these changes Aug 2, 2024

View reviewed changes

michenly and others added 3 commits August 2, 2024 11:26

Update packages/hydrogen/src/createHydrogenContext.ts

cd772b1

Co-authored-by: Bret Little <[email protected]>

Update packages/hydrogen/src/createHydrogenContext.doc.ts

ff0b158

Co-authored-by: Bret Little <[email protected]>

Update packages/cli/assets/i18n/mock-i18n-types.ts

a5748d5

Co-authored-by: Bret Little <[email protected]>

frandiox approved these changes Aug 2, 2024

View reviewed changes

michenly added 7 commits August 2, 2024 14:21

update changelog

441c3fe

update context path in i18n replacer to use joinPath

31faaaa

simplify mock-i18n-type replacement

0ab9cd8

add reasoning for HydrogenContextOverloads

4d3a4f3

remove duplicate env.session check

7823caf

i18n strategy update

855657d

update i18n test

688fd07

michenly merged commit bf4e3d3 into main Aug 2, 2024
13 checks passed

michenly deleted the mc-create-shopify branch August 2, 2024 21:45

shopify-github-actions-access bot mentioned this pull request Aug 2, 2024

[ci] release 2024-07 #2378

Merged

		// update imported type from mock-i18n-types to @shopify/hydrogen with TS file
		const importFromNode = root.find({

Create createHydrogenContext that combine createStorefrontClient,createCustomerAccountClient and createCartHandler #2333

Create createHydrogenContext that combine createStorefrontClient,createCustomerAccountClient and createCartHandler #2333

Conversation

michenly commented Jul 15, 2024 • edited Loading

Default usage

With customized cart methods

This comment has been minimized.

shopify bot commented Jul 15, 2024 • edited Loading

frandiox left a comment

Choose a reason for hiding this comment

frandiox Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frandiox Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michenly Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michenly Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michenly commented Jul 30, 2024

github-actions bot commented Jul 30, 2024

michenly commented Jul 30, 2024

frandiox commented Jul 31, 2024

michenly commented Jul 31, 2024

michenly commented Jul 31, 2024

blittle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michenly Aug 2, 2024 • edited Loading

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

frandiox 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Create `createHydrogenContext` that combine `createStorefrontClient`,`createCustomerAccountClient` and `createCartHandler` #2333

Create `createHydrogenContext` that combine `createStorefrontClient`,`createCustomerAccountClient` and `createCartHandler` #2333

michenly commented Jul 15, 2024 •

edited

Loading

shopify bot commented Jul 15, 2024 •

edited

Loading

frandiox Jul 18, 2024 •

edited

Loading

frandiox Jul 29, 2024 •

edited

Loading

michenly Jul 18, 2024 •

edited

Loading

michenly Jul 29, 2024 •

edited

Loading

michenly Aug 2, 2024 •

edited

Loading