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

[3.73] Fix shopify theme dev to no longer fail when development the… #5202

Merged
merged 2 commits into from
Jan 16, 2025
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/lemon-pants-add.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/cli-kit': patch
'@shopify/theme': patch
---

Fix `shopify theme dev` to no longer fail when development themes expire in internationalized stores
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ export type ThemeRole =
* publishing are restricted until the merchant resolves the licensing issue.
*/
| 'LOCKED'
/** TThe currently published theme. There can only be one main theme at any time. */
/** The currently published theme. There can only be one main theme at any time. */
| 'MAIN'
/** The currently published theme that is only accessible to a mobile client. */
| 'MOBILE'
Expand Down
2 changes: 1 addition & 1 deletion packages/cli-kit/src/public/node/themes/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('fetchTheme', () => {
test('returns undefined when a theme is not found', async () => {
const errorResponse = {
status: 200,
errors: [{message: 'Theme does not exist'} as any],
errors: [{message: 'Tema não existe'} as any],
}
vi.mocked(adminRequestDoc).mockRejectedValue(new ClientError(errorResponse, {query: ''}))

Expand Down
39 changes: 21 additions & 18 deletions packages/cli-kit/src/public/node/themes/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,33 +25,36 @@ import {buildTheme} from '@shopify/cli-kit/node/themes/factories'
import {Result, Checksum, Key, Theme, ThemeAsset, Operation} from '@shopify/cli-kit/node/themes/types'
import {outputDebug} from '@shopify/cli-kit/node/output'
import {sleep} from '@shopify/cli-kit/node/system'
import {ClientError} from 'graphql-request'

export type ThemeParams = Partial<Pick<Theme, 'name' | 'role' | 'processing' | 'src'>>
export type AssetParams = Pick<ThemeAsset, 'key'> & Partial<Pick<ThemeAsset, 'value' | 'attachment'>>

export async function fetchTheme(id: number, session: AdminSession): Promise<Theme | undefined> {
const gid = composeThemeGid(id)

try {
const response = await adminRequestDoc(GetTheme, session, {id: composeThemeGid(id)}, undefined, {
const {theme} = await adminRequestDoc(GetTheme, session, {id: gid}, undefined, {
handleErrors: false,
})
const {theme} = response
if (!theme) {
return undefined
}
return buildTheme({
id: parseGid(theme.id),
processing: theme.processing,
role: theme.role.toLowerCase(),
name: theme.name,
})
} catch (error) {
if (error instanceof ClientError) {
if (error.response?.errors?.[0]?.message === 'Theme does not exist') {
return undefined
}

if (theme) {
return buildTheme({
id: parseGid(theme.id),
processing: theme.processing,
role: theme.role.toLowerCase(),
name: theme.name,
})
}
throw new AbortError(`Failed to fetch theme: ${id}`)

// eslint-disable-next-line no-catch-all/no-catch-all
} catch (_error) {
/**
* Consumers of this and other theme APIs in this file expect either a theme
* or `undefined`.
*
* Error handlers should not inspect GraphQL error messages directly, as
* they are internationalized.
*/
}
}

Expand Down
Loading