Client-side analytics integration in demo-store #376
Conversation
wizardlyhel
changed the title
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Josh Larson <[email protected]>
Co-authored-by: Josh Larson <[email protected]>
| export type I18nBase = { | ||
| language: LanguageCode; | ||
| country: CountryCode; | ||
| }; | ||
|
|
||
| export type CreateStorefrontClientOptions<TI18n extends I18nBase> = Parameters< | ||
| typeof createStorefrontUtilities | ||
| >[0] & { | ||
| cache?: Cache; | ||
| buyerIp?: string; | ||
| requestGroupId?: string; | ||
| waitUntil?: ExecutionContext['waitUntil']; | ||
| i18n?: { | ||
| language: LanguageCode; | ||
| country: CountryCode; | ||
| pathPrefix?: string; | ||
| }; | ||
| i18n?: TI18n; |
There was a problem hiding this comment.
@wizardlyhel I'm a bit late to the party but what's the reason to remove pathPrefix from here? We are still passing it in server.ts as a parameter to createStorefrontClient({ i18n: {pathPrefix...}}).
I guess it's because we don't use that internally in Hydrogen, so now it's only part of the types in the user land? -- like, they need to cast the type when reading pathPrefix in loaders. Is that correct?
There was a problem hiding this comment.
No, they shouldn't need to typecast ... It should be the type that they return from their i18n function.
createStorefrontClient shouldn't dictate the type of i18n. Developers should dictate what that type is.
Let say we leave it what it was before and developer would like to have more i18n related properties (ie. currency) to be pass along in the i18n object, they would end up typecasting anyways.
So the key is that we need to make sure the type is being pass thru the createStorefrontClient.. but I just found out the type isn't being passed over (I was testing the type generic and it works there. Need to figure out what happen)
There was a problem hiding this comment.
I can see it working inside server.ts
Example integration of Shopify analytics using
storefront-kit-reactDocs: