Skip to content
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/nice-radios-lie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/cli-kit': patch
'@shopify/theme': patch
---

Extract the ownership of development themes
2 changes: 2 additions & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ module.exports = {
'**/public/node/plugins/tunnel.ts',
'**/public/node/environments.ts',
'**/public/node/result.ts',
'**/public/node/themes/**/*',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

],
rules: {
'jsdoc/check-access': 'error',
Expand Down Expand Up @@ -251,6 +252,7 @@ module.exports = {
rules: {
'@typescript-eslint/explicit-module-boundary-types': 'error',
},
excludedFiles: ['**/public/node/themes/**/*'],
},
{
files: ['src/private/node/ui/components/**/*.tsx'],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import {generateThemeName} from './generate-theme-name.js'
import {beforeEach, describe, expect, it, vi} from 'vitest'
import {hostname} from 'os'
import {randomBytes} from 'crypto'

vi.mock('os')
vi.mock('crypto')

describe('generateThemeName', () => {
const context = 'Development'

beforeEach(() => {
vi.mocked(randomBytes).mockImplementation(() => Buffer.from([1, 2, 3]))
})

it('should not truncate if the theme name is below the API limit', () => {
vi.mocked(hostname).mockReturnValue('Mac-Book-Pro.My-Router')
expect(generateThemeName(context)).toEqual('Development (010203-Mac-Book-Pro)')
})

it('should truncate if the theme name is above the API limit', () => {
vi.mocked(hostname).mockReturnValue('theme-dev-lan-very-long-name-that-will-be-truncated')
expect(generateThemeName(context)).toEqual('Development (010203-theme-dev-lan-very-long-name-t)')
})
})
15 changes: 15 additions & 0 deletions packages/cli-kit/src/public/node/themes/generate-theme-name.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import {replaceInvalidCharacters} from './replace-invalid-characters.js'
import {randomBytes} from 'crypto'
import {hostname} from 'os'

const API_NAME_LIMIT = 50

export function generateThemeName(context: string): string {
const hostNameWithoutDomain = hostname().split('.')[0]!
const hash = randomBytes(3).toString('hex')

const name = `${context} ()`
const hostNameCharacterLimit = API_NAME_LIMIT - name.length - hash.length
const identifier = replaceInvalidCharacters(`${hash}-${hostNameWithoutDomain.substring(0, hostNameCharacterLimit)}`)
return `${context} (${identifier})`
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
export const DEVELOPMENT_THEME_ROLE = 'development'

export class Theme {
constructor(public id: number, public name: string, private _role: string) {}
constructor(public id: number, public name: string, private _role: string, public createdAtRuntime = false) {}

get role(): string {
if (this._role === 'main') {
Expand All @@ -16,4 +18,8 @@ export class Theme {
this._role = _role
}
}

get hasDevelopmentRole(): boolean {
return this.role === DEVELOPMENT_THEME_ROLE
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import {replaceInvalidCharacters} from './replace-invalid-characters.js'
import {describe, expect, it} from 'vitest'

describe('replaceInvalidCharacters', () => {
it('should replace unused ASCII characters', () => {
const asciiStringChar = '\x8F'
expect(replaceInvalidCharacters(`theme-dev-${asciiStringChar}.lan`)).toEqual('theme-dev---lan')
})

it('should not replace non-latin letters and marks', () => {
const hostName = 'ÇaVaこんにちはПривіт'
expect(replaceInvalidCharacters(hostName)).toEqual(hostName)
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export function replaceInvalidCharacters(identifier: string) {
const findAllMatches = 'g'
const enablesUnicodeSupport = 'u'
return identifier.replace(
new RegExp(/[^\p{Letter}\p{Number}\p{Mark}-]/, `${findAllMatches}${enablesUnicodeSupport}`),
'-',
)
}
50 changes: 50 additions & 0 deletions packages/cli-kit/src/public/node/themes/theme-manager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import {fetchTheme, createTheme} from './themes-api.js'
import {DEVELOPMENT_THEME_ROLE, Theme} from './models/theme.js'
import {generateThemeName} from './generate-theme-name.js'
import {AdminSession} from '@shopify/cli-kit/node/session'
import {BugError} from '@shopify/cli-kit/node/error'

export abstract class ThemeManager {
protected themeId: string | undefined
protected abstract setTheme(themeId: string): void
protected abstract removeTheme(): void
protected abstract context: string

constructor(protected adminSession: AdminSession) {}

async findOrCreate(): Promise<Theme> {
let theme = await this.fetch()
if (!theme) {
theme = await this.create()
}
return theme
}

protected async fetch() {
if (!this.themeId) {
return
}
const theme = await fetchTheme(parseInt(this.themeId, 10), this.adminSession)
if (!theme) {
this.removeTheme()
}
return theme
}

private async create() {
const name = generateThemeName(this.context)
const role = DEVELOPMENT_THEME_ROLE
const theme = await createTheme(
{
name,
role,
},
this.adminSession,
)
if (!theme) {
throw new BugError(`Could not create theme with name "${name}" and role "${role}"`)
}
this.setTheme(theme.id.toString())
return theme
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {storeAdminUrl, themeEditorUrl, themePreviewUrl} from './theme-urls.js'
import {Theme} from '../models/theme.js'
import {Theme} from './models/theme.js'
import {test, describe, expect} from 'vitest'

const session = {token: 'token', storeFqdn: 'my-shop.myshopify.com'}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Theme} from '../models/theme.js'
import {Theme} from '@shopify/cli-kit/node/themes/models/theme'
import {AdminSession} from '@shopify/cli-kit/node/session'

export function themePreviewUrl(theme: Theme, session: AdminSession) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,26 @@ import * as throttler from './themes-api/throttler.js'
import {apiCallLimit, retryAfter} from './themes-api/headers.js'
import {retry} from './themes-api/retry.js'
import {storeAdminUrl} from './theme-urls.js'
import {Theme} from '../models/theme.js'
import {Theme} from './models/theme.js'
import {restRequest, RestResponse} from '@shopify/cli-kit/node/api/admin'
import {AdminSession} from '@shopify/cli-kit/node/session'
import {AbortError} from '@shopify/cli-kit/node/error'

export type ThemeParams = Partial<Pick<Theme, 'name' | 'role'>>

export async function fetchTheme(id: number, session: AdminSession): Promise<Theme | undefined> {
const response = await request('GET', `/themes/${id}`, session, undefined, {fields: 'id'})
return buildTheme(response.json.theme)
}

export async function fetchThemes(session: AdminSession): Promise<Theme[]> {
const response = await request('GET', '/themes', session, undefined, {fields: 'id,name,role'})
return buildThemes(response)
}

export async function createTheme(params: ThemeParams, session: AdminSession): Promise<Theme | undefined> {
const response = await request('POST', '/themes', session, {theme: {...params}})
return buildTheme(response.json.theme)
return buildTheme({...response.json.theme, createdAtRuntime: true})
}

export async function updateTheme(id: number, params: ThemeParams, session: AdminSession): Promise<Theme | undefined> {
Expand Down Expand Up @@ -87,7 +92,7 @@ function buildTheme(themeJson: any): Theme | undefined {
return undefined
}

return new Theme(themeJson.id, themeJson.name, themeJson.role)
return new Theme(themeJson.id, themeJson.name, themeJson.role, themeJson.createdAtRuntime)
}

function handleForbiddenError(session: AdminSession): never {
Expand Down
2 changes: 1 addition & 1 deletion packages/theme/oclif.manifest.json

Large diffs are not rendered by default.

28 changes: 20 additions & 8 deletions packages/theme/src/cli/commands/theme/dev.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {themeFlags} from '../../flags.js'
import {ensureThemeStore} from '../../utilities/theme-store.js'
import ThemeCommand from '../../utilities/theme-command.js'
import {DevelopmentThemeManager} from '../../utilities/development-theme-manager.js'
import {Flags} from '@oclif/core'
import {globalFlags} from '@shopify/cli-kit/node/cli'
import {execCLI2} from '@shopify/cli-kit/node/ruby'
import {AbortController} from '@shopify/cli-kit/node/abort'
import {ensureAuthenticatedStorefront, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session'
import {AdminSession, ensureAuthenticatedStorefront, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session'
import {sleep} from '@shopify/cli-kit/node/system'
import {outputDebug} from '@shopify/cli-kit/node/output'

Expand Down Expand Up @@ -80,6 +81,7 @@ export default class Dev extends ThemeCommand {
'live-reload',
'poll',
'theme-editor-sync',
'overwrite-json',
'port',
'theme',
'only',
Expand All @@ -96,30 +98,40 @@ export default class Dev extends ThemeCommand {
* Every 110 minutes, it will refresh the session token and restart the server.
*/
async run(): Promise<void> {
const {flags} = await this.parse(Dev)
let {flags} = await this.parse(Dev)
const store = ensureThemeStore(flags)
const adminSession = await ensureAuthenticatedThemes(store, flags.password, [], true)
const theme = await new DevelopmentThemeManager(adminSession).findOrCreate()
flags = {
...flags,
theme: theme.id.toString(),
'overwrite-json': Boolean(flags['theme-editor-sync']) && theme.createdAtRuntime,
}

const flagsToPass = this.passThroughFlags(flags, {allowedFlags: Dev.cli2Flags})
const command = ['theme', 'serve', flags.path, ...flagsToPass]

const store = ensureThemeStore(flags)

let controller = new AbortController()

setInterval(() => {
outputDebug('Refreshing theme session token and restarting theme server...')
controller.abort()
controller = new AbortController()
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.execute(store, flags.password, command, controller)
this.execute(adminSession, flags.password, command, controller)
}, this.ThemeRefreshTimeoutInMs)

// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.execute(store, flags.password, command, controller)
this.execute(adminSession, flags.password, command, controller)
}

async execute(store: string, password: string | undefined, command: string[], controller: AbortController) {
async execute(
adminSession: AdminSession,
password: string | undefined,
command: string[],
controller: AbortController,
) {
await sleep(3)
const adminSession = await ensureAuthenticatedThemes(store, password, [], true)
const storefrontToken = await ensureAuthenticatedStorefront([], password)
return execCLI2(command, {adminSession, storefrontToken, signal: controller.signal})
}
Expand Down
16 changes: 13 additions & 3 deletions packages/theme/src/cli/commands/theme/pull.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {themeFlags} from '../../flags.js'
import {ensureThemeStore} from '../../utilities/theme-store.js'
import ThemeCommand from '../../utilities/theme-command.js'
import {DevelopmentThemeManager} from '../../utilities/development-theme-manager.js'
import {Flags} from '@oclif/core'
import {globalFlags} from '@shopify/cli-kit/node/cli'
import {execCLI2} from '@shopify/cli-kit/node/ruby'
Expand Down Expand Up @@ -56,7 +57,18 @@ export default class Pull extends ThemeCommand {
static cli2Flags = ['theme', 'development', 'live', 'nodelete', 'only', 'ignore', 'force']

async run(): Promise<void> {
const {flags} = await this.parse(Pull)
let {flags} = await this.parse(Pull)
const store = ensureThemeStore(flags)
const adminSession = await ensureAuthenticatedThemes(store, flags.password)

if (flags.development) {
const theme = await new DevelopmentThemeManager(adminSession).find()
flags = {
...flags,
development: false,
theme: theme.id.toString(),
}
}

let validPath = flags.path
if (!isAbsolutePath(validPath)) {
Expand All @@ -67,8 +79,6 @@ export default class Pull extends ThemeCommand {

const command = ['theme', 'pull', validPath, ...flagsToPass]

const store = ensureThemeStore(flags)
const adminSession = await ensureAuthenticatedThemes(store, flags.password)
await execCLI2(command, {adminSession})
}
}
16 changes: 13 additions & 3 deletions packages/theme/src/cli/commands/theme/push.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {themeFlags} from '../../flags.js'
import {ensureThemeStore} from '../../utilities/theme-store.js'
import ThemeCommand from '../../utilities/theme-command.js'
import {DevelopmentThemeManager} from '../../utilities/development-theme-manager.js'
import {Flags} from '@oclif/core'
import {globalFlags} from '@shopify/cli-kit/node/cli'
import {execCLI2} from '@shopify/cli-kit/node/ruby'
Expand Down Expand Up @@ -95,13 +96,22 @@ export default class Push extends ThemeCommand {
]

async run(): Promise<void> {
const {flags} = await this.parse(Push)
let {flags} = await this.parse(Push)
const store = ensureThemeStore(flags)
const adminSession = await ensureAuthenticatedThemes(store, flags.password)

if (flags.development) {
const theme = await new DevelopmentThemeManager(adminSession).findOrCreate()
flags = {
...flags,
development: false,
theme: theme.id.toString(),
}
}

const flagsToPass = this.passThroughFlags(flags, {allowedFlags: Push.cli2Flags})
const command = ['theme', 'push', flags.path, ...flagsToPass]

const store = ensureThemeStore(flags)
const adminSession = await ensureAuthenticatedThemes(store, flags.password)
await execCLI2(command, {adminSession})
}
}
Loading