Consolidate Hydrogen config options and explore partner integration story

[jplhomer]

This is a blanket ticket to cover work related to consolidating Hydrogen configs and exploring what it's like for third parties to integrate with Hydrogen.

We currently accept runtime options a number of ways:

• renderHydrogen in a custom client entrypoint for enabling/disabling strict mode
• renderHydrogen in App.server.jsx for routes and shopifyConfig
• Logger options in App.server.jsx
• Dev caching options and shopifyConfig again in hydrogen() Vite plugin

Additionally, these options all require changes to source code and can't be enabled with CLI flags, etc. Some of them might be useful as CLI flags, some not.

We need to explore consolidating these options, perhaps by introducing a hydrogen.config.js file, or moving them to a single location.

We should also take this opportunity to determine if and how this type of config consolidation plays into a larger partner integration story.

• If I'm Sanity and I'm providing a few useful utilities, is there a dedicated place to inject public or private configs?
• If I'm Partytown or Builder.io, how can I easily integrate with Hydrogen?
• If I'm a third-party analytics adapter, how can I integrate with Hydrogen? #890

Related: #910

[jplhomer]

Current ideas:

Option 1: Use App.server.jsx

This means the developer provides options to renderHydrogen. We can be smart about things and proxy client options (like strictMode) to the correct places, so the developer doesn't have to eject a client entrypoint.

// App.server.jsx

export default renderHydrogen(
  App,
  {
    routes,
    shopifyConfig,

    // This proxies through to the client entrypoint somehow
    strictMode: false,

    // Logger options
    logger: {
      showCacheApiStatus: true,
      showCacheControlHeader: true,
      showQueryTiming: true,
      warn(request, ...args) { ... }
    }

    // 3P Plugins could go here
    plugins: [
      hydrogenSanity({ config: sanityConfig })
    ]
  }
);

Pros:

• Everything is in one place
• No more setting log config in a singleton
• Settings can be modified on a per-request basis

Cons:

• This is at the bottom of a React component file. Imagine if you were a 3p plugin advertising how to install on Hydrogen, and how confusing it might be to say "First, open up your App.server.jsx file and scroll to the bottom and find renderHydrogen and add this property to object in the second argument of that function." Not elegant.
• Options are nested one additional layer (since they are passed as arguments to a function).

Option 2: Use hydrogen.config.js

This option introduces another config file called hydrogen.config.js. This is a special file to the Hydrogen framework, not to be confused with shopify.config.js which is strictly for Shopify credentials.

// hydrogen.config.js

import shopifyConfig from './shopify.config';

export default {
  routes: import.meta.globEager('./routes/**/*.server.[jt](s|sx)'),
  shopifyConfig,

  // This proxies through to the client entrypoint somehow
  strictMode: false,

  // Logger options
  logger: {
    showCacheApiStatus: true,
    showCacheControlHeader: true,
    showQueryTiming: true,
    warn(request, ...args) {
      // ...
    },
  },

  // 3P Plugins could go here
  plugins: [hydrogenSanity({config: sanityConfig})],
};

Pros:

• One less layer of nesting, not passed as an arg to a function anymore
• We could probably remove export default renderHydrogen from App.server.jsx altogether, and teach Hydrogen to kick off the server entrypoint based on the contents of this file (or an internal default)
• ~Optional, and devs can add it as needed~ Hmm unfortunately, we can't make the routes optional due to restrictions around import meta glob. Or can we?

Cons:

• Yet another config file, in addition to Vite, Shopify, PostCSS, Tailwind, etc.
• Could be confusing that we have both a shopify.config.js and a hydrogen.config.js. For example, should we move defaultLocale to be an option here instead of in shopify.config.js?

[blittle]

I think I prefer option 2. It is unfortunate to have another config, but hydrogen is the metaframework, and it's pretty standard for a metaframework to own a config file. I think we should draw the line that shopifiy.config.js should only be for credentials. Everything else like locale should be in hydrogen.config.js

[frehner]

I like option 2, but also... I don't really think we need to have the separation between shopify.config.js and hydrogen.config.js. Every other metaframework consolidates their config into one spot, and I think it is unnecessarily confusing to have two configs (both for maintainers and for developers). Is there a good reason why we wouldn't combine them?

[wizardlyhel]

also like the hydrogen.config.js

I don't think we can merge shopify.config.js since it is needed for vite.config.js but maybe we can make it 2 named exports?

import {ShopifyConfig, HydrogenConfig} from 'hydrogen.config'

but that is a breaking change

[frehner]

also like the hydrogen.config.js

I don't think we can merge shopify.config.js since it is needed for vite.config.js but maybe we can make it 2 named exports?

import {ShopifyConfig, HydrogenConfig} from 'hydrogen.config'

but that is a breaking change

Or nest objects, e.g.

// hydrogen.config.js
{
  storefrontApi: {
    token: ''
    domain: ''
    version: ''
  },
  logger: {}
  ...
}

[tobi]

i think its fine to have breaking changes at this point as long as we fail fast and with good error ( which should be straightforward here because we can detect the missing keys )

[cartogram]

Option 2 + @frehner's suggestion of grouping the storefront object within seems like a good approach to me.

Some of them might be useful as CLI flags, some not.

I think we can get away with a --config path/to/config for the most part (outside of maybe logging options, which could make sense as top-level CLI flags at some point). cc @benjaminsehl

As part of this work we can easily just provide a defineConfig({...}) utility to help with intellisense and type-checking to catch breaking changes and fail fast.

[frandiox]

DX-wise I would prefer option 2. However, we might find some issues with that:

• We've seen some use cases where the shopifyConfig passed to the app must change depending on the URL/request. This becomes harder to customize when we are the ones importing from the file instead of them.
• import.meta for reading environment variables or routes wouldn't work if we merge hydrogen.config with shopify.config, because the latter must be imported in the plugin (outside of Vite). process.env can be used instead, but the .env files are still not read at this point.

As a possible solution, perhaps we can do option 2 by default but still allow overwriting it with option 1 -- i.e. if the developer passes options to renderHydrogen, we use that instead of reading from the file (or merge them à la Vite). Alternatively, hydrogen.config can export a function that we call on every request to get the config (HydrogenConfig | (req: Request) => HydrogenConfig).

Also, we can do the same in the plugin so that developers don't pass shopifyConfig directly. Instead, we import it ourselves and make sure .env files and import.meta are available 🤔

[b-barry]

Hello all,

With the plugin system, do you plan to allow the merging in the hydrogenConfig? For example for hydrogenSanity, I would like to have a sanity node in in the hydrogenConfig to scope all data related to sanity like for the shopify. Also, could we have a hook such as useHydrogenConfig that provide the complete hydrogenConfig because useShop provide only the shopify config

[frandiox]

@b-barry Rather than providing sanity data at the top level, I think the following could work better with typings and other extensions:

import {defineConfig} from '@shopify/hydrogen/config';
import {sanity} from '...';

export default defineConfig({
  shopify: { ... },
  plugins: [sanity({ /* Sanity options and data */ })],
})

Also, could we have a hook such as useHydrogenConfig that provide the complete hydrogenConfig

Can you explain more about your use case? Is this only for getting Sanity data in this example, or something else?

[b-barry]

@b-barry Rather than providing sanity data at the top level, I think the following could work better with typings and other extensions:

import {defineConfig} from '@shopify/hydrogen/config';
import {sanity} from '...';

export default defineConfig({
  shopify: { ... },
  plugins: [sanity({ /* Sanity options and data */ })],
})

@frandiox could you elaborate how the config object will handled/merged with the plugins? I agree completely agree that for the typing it is better. We should use interface for making merging easier

Also, could we have a hook such as useHydrogenConfig that provide the complete hydrogenConfig

Can you explain more about your use case? Is this only for getting Sanity data in this example, or something else?

I mean it will be for getting any data stored in the configuration. Every plugins could provide their hook such as useSanityConfig but I think they should all use a low level hook. At the moment, I am adding some custom property without respecting the typing for testing purpose. I don't have any way to retrieve them. I know it is bad haha but I am just playing around

[frandiox]

could you elaborate how the config object will handled/merged with the plugins? I agree completely agree that for the typing it is better. We should use interface for making merging easier

This is still not decided but I don't think plugins would just be merged with the config property by property. However, they will have a way to provide data to the app, I'm fairly sure of that. In fact, we already have useRequestContext hook that can be used for this so the idea is that a plugin can provide context to that hook.

// Not actual API, just a quick draft
  name: 'my-plugin',
  middleware: ({request, context}) => {
    if (request.url.startsWith('/...') {
      context.data = 'my-data';
    }
  }
}

...

// In-app
useRequestContext('my-plugin').data // 'my-data'

[b-barry]

could you elaborate how the config object will handled/merged with the plugins? I agree completely agree that for the typing it is better. We should use interface for making merging easier

This is still not decided but I don't think plugins would just be merged with the config property by property. However, they will have a way to provide data to the app, I'm fairly sure of that. In fact, we already have useRequestContext hook that can be used for this so the idea is that a plugin can provide context to that hook.

// Not actual API, just a quick draft
  name: 'my-plugin',
  middleware: ({request, context}) => {
    if (request.url.startsWith('/...') {
      context.data = 'my-data';
    }
  }
}

...

// In-app
useRequestContext('my-plugin').data // 'my-data'

I didn't know this hook. It will solve a lot of issue :) ( I am getting a Suspense waterfall detected when following the doc btw). Thanks for the clarification. Only concern that I see it we should not use name as string to avoid any future collision. For example, two plugin could use the same name without knowing it. What do you think about using Symbol?