-
Notifications
You must be signed in to change notification settings - Fork 231
Create abstract client steps infrastracture to externalize the ap modules client configurations #6965
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
Open
alfonso-noriega
wants to merge
1
commit into
03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications
Choose a base branch
from
03-10-create_abstract_client_steps_infrastracture_to_externalize_the_ap_modules_client_configurations
base: 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Create abstract client steps infrastracture to externalize the ap modules client configurations #6965
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| import {executeStep, BuildContext, LifecycleStep} from './client-steps.js' | ||
| import * as stepsIndex from './steps/index.js' | ||
| import {ExtensionInstance} from '../../models/extensions/extension-instance.js' | ||
| import {beforeEach, describe, expect, test, vi} from 'vitest' | ||
|
|
||
| vi.mock('./steps/index.js') | ||
|
|
||
| describe('executeStep', () => { | ||
| let mockContext: BuildContext | ||
|
|
||
| beforeEach(() => { | ||
| mockContext = { | ||
| extension: { | ||
| directory: '/test/dir', | ||
| outputPath: '/test/output/index.js', | ||
| } as ExtensionInstance, | ||
| options: { | ||
| stdout: {write: vi.fn()} as any, | ||
| stderr: {write: vi.fn()} as any, | ||
| app: {} as any, | ||
| environment: 'production' as const, | ||
| }, | ||
| stepResults: new Map(), | ||
| } | ||
| }) | ||
|
|
||
| const step: LifecycleStep = { | ||
| id: 'test-step', | ||
| name: 'Test Step', | ||
| type: 'include_assets', | ||
| config: {}, | ||
| } | ||
|
|
||
| describe('success', () => { | ||
| test('returns a successful StepResult with output', async () => { | ||
| vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({filesCopied: 3}) | ||
|
|
||
| const result = await executeStep(step, mockContext) | ||
|
|
||
| expect(result.id).toBe('test-step') | ||
| expect(result.success).toBe(true) | ||
| if (result.success) expect(result.output).toEqual({filesCopied: 3}) | ||
| expect(result.duration).toBeGreaterThanOrEqual(0) | ||
| }) | ||
|
|
||
| test('logs step execution to stdout', async () => { | ||
| vi.mocked(stepsIndex.executeStepByType).mockResolvedValue({}) | ||
|
|
||
| await executeStep(step, mockContext) | ||
|
|
||
| expect(mockContext.options.stdout.write).toHaveBeenCalledWith('Executing step: Test Step\n') | ||
| }) | ||
| }) | ||
|
|
||
| describe('failure', () => { | ||
| test('throws a wrapped error when the step fails', async () => { | ||
| vi.mocked(stepsIndex.executeStepByType).mockRejectedValue(new Error('something went wrong')) | ||
|
|
||
| await expect(executeStep(step, mockContext)).rejects.toThrow( | ||
| 'Build step "Test Step" failed: something went wrong', | ||
| ) | ||
| }) | ||
|
|
||
| test('returns a failure result and logs a warning when continueOnError is true', async () => { | ||
| vi.mocked(stepsIndex.executeStepByType).mockRejectedValue(new Error('something went wrong')) | ||
|
|
||
| const result = await executeStep({...step, continueOnError: true}, mockContext) | ||
|
|
||
| expect(result.success).toBe(false) | ||
| if (!result.success) expect(result.error?.message).toBe('something went wrong') | ||
| expect(mockContext.options.stderr.write).toHaveBeenCalledWith( | ||
| 'Warning: Step "Test Step" failed but continuing: something went wrong\n', | ||
| ) | ||
| }) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import {executeStepByType} from './steps/index.js' | ||
| import type {ExtensionInstance} from '../../models/extensions/extension-instance.js' | ||
| import type {ExtensionBuildOptions} from './extension.js' | ||
|
|
||
| /** | ||
| * LifecycleStep represents a single step in the client-side build pipeline. | ||
| * Pure configuration object — execution logic is separate (router pattern). | ||
| */ | ||
| export interface LifecycleStep { | ||
| /** Unique identifier, used as the key in the stepResults map */ | ||
| readonly id: string | ||
|
|
||
| /** Human-readable name for logging */ | ||
| readonly name: string | ||
|
|
||
| /** Step type (determines which executor handles it) */ | ||
| readonly type: | ||
| | 'include_assets' | ||
| | 'build_theme' | ||
| | 'bundle_theme' | ||
| | 'bundle_ui' | ||
| | 'copy_static_assets' | ||
| | 'build_function' | ||
| | 'create_tax_stub' | ||
|
|
||
| /** Step-specific configuration */ | ||
| readonly config: {[key: string]: unknown} | ||
|
|
||
| /** Whether to continue on error (default: false) */ | ||
| readonly continueOnError?: boolean | ||
| } | ||
|
|
||
| /** | ||
| * A group of steps scoped to a specific lifecycle phase. | ||
| * Allows executing only the steps relevant to a given lifecycle (e.g. 'deploy'). | ||
| */ | ||
| interface ClientLifecycleGroup { | ||
| readonly lifecycle: 'deploy' | ||
| readonly steps: ReadonlyArray<LifecycleStep> | ||
| } | ||
|
|
||
| /** | ||
| * The full client steps configuration for an extension. | ||
| * Replaces the old buildConfig contract. | ||
| */ | ||
| export type ClientSteps = ReadonlyArray<ClientLifecycleGroup> | ||
alfonso-noriega marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * Context passed through the step pipeline. | ||
| * Each step can read from and write to the context. | ||
| */ | ||
| export interface BuildContext { | ||
alfonso-noriega marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| readonly extension: ExtensionInstance | ||
| readonly options: ExtensionBuildOptions | ||
| readonly stepResults: Map<string, StepResult> | ||
| } | ||
|
|
||
| type StepResult = { | ||
| readonly id: string | ||
| readonly duration: number | ||
| } & ( | ||
| | { | ||
| readonly success: false | ||
| readonly error: Error | ||
| } | ||
| | { | ||
| readonly success: true | ||
| readonly output: never | ||
| } | ||
| ) | ||
|
|
||
| /** | ||
| * Executes a single client step with error handling. | ||
| */ | ||
| export async function executeStep(step: LifecycleStep, context: BuildContext): Promise<StepResult> { | ||
| const startTime = Date.now() | ||
|
|
||
| try { | ||
| context.options.stdout.write(`Executing step: ${step.name}\n`) | ||
| const output = await executeStepByType(step, context) | ||
|
|
||
| return { | ||
| id: step.id, | ||
| success: true, | ||
| duration: Date.now() - startTime, | ||
| output: output as never, | ||
| } | ||
alfonso-noriega marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } catch (error) { | ||
| const stepError = error as Error | ||
|
|
||
| if (step.continueOnError) { | ||
| context.options.stderr.write(`Warning: Step "${step.name}" failed but continuing: ${stepError.message}\n`) | ||
| return { | ||
| id: step.id, | ||
| success: false, | ||
| duration: Date.now() - startTime, | ||
| error: stepError, | ||
| } | ||
| } | ||
|
|
||
| throw new Error(`Build step "${step.name}" failed: ${stepError.message}`) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import type {LifecycleStep, BuildContext} from '../client-steps.js' | ||
|
|
||
| /** | ||
| * Routes step execution to the appropriate handler based on step type. | ||
| * This implements the Command Pattern router, dispatching to type-specific executors. | ||
| * | ||
| * @param step - The build step configuration | ||
| * @param context - The build context | ||
| * @returns The output from the step execution | ||
| * @throws Error if the step type is not implemented or unknown | ||
| */ | ||
| export async function executeStepByType(step: LifecycleStep, _context: BuildContext): Promise<unknown> { | ||
| switch (step.type) { | ||
| // Future step types (not implemented yet): | ||
| case 'include_assets': | ||
| case 'build_theme': | ||
| case 'bundle_theme': | ||
| case 'bundle_ui': | ||
| case 'copy_static_assets': | ||
| case 'build_function': | ||
| case 'create_tax_stub': | ||
| throw new Error(`Build step type "${step.type}" is not yet implemented.`) | ||
alfonso-noriega marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| default: | ||
| throw new Error(`Unknown build step type: ${(step as {type: string}).type}`) | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.