Skip to content

feat(yeoman-UI): backward compatible progress notifications - #613

Merged
ilya-taupeka merged 16 commits into
SAP:mainfrom
korotkovao:feat/yeoman-ui/backward-compatible-progress-notifications
Sep 24, 2026
Merged

ilya-taupeka merged 16 commits into
SAP:mainfrom
korotkovao:feat/yeoman-ui/backward-compatible-progress-notifications

Conversation

@korotkovao

Copy link
Copy Markdown
Contributor

feat(backend): implement backward-compatible progress notifications

Summary

Implements enhanced multi-phase progress notifications for Yeoman generators with zero breaking changes. Generators that don't opt in still receive the classic "Installing dependencies..." toast.

Based on reverted PR #576 with critical backward compatibility improvements.

Changes

New Features

  • doGeneratorProgress() method: Multi-phase progress tracking for opt-in generators
    • Phase 1: "Creating project files..." (writing phase)
    • Phase 2: "Installing dependencies..." (install phase)
    • Phase 3: "Finalising..." (end phase)
  • Opt-in mechanism: Generators set showGeneratorProgress: true option to enable enhanced progress
  • Minimum phase durations: Ensures each phase is visible (writing: 2s, end: 1s)

Backward Compatibility

  • doGeneratorInstall() preserved: Classic behavior unchanged
  • Default behavior: showProgress: false - generators without opt-in get classic single-phase toast
  • Project name extraction: Works with multiple generator state locations for broad compatibility

Bug Fixes (from code review)

  • Fixed enhanced mode flag not set if generator skips "writing" phase
  • Fixed setTimeout race condition between concurrent generators
  • Fixed state persistence between generator runs
  • Fixed duplicate message display in classic mode

Testing

  • ✅ Classic mode tested: Single "Installing dependencies..." toast (no opt-in)
  • ✅ Enhanced mode tested: 3-phase progress with project name
  • ✅ All 294 tests passing
  • ✅ Lint passing

Breaking Changes

None - fully backward compatible.

Related

…ications

Implements enhanced multi-phase progress notifications for Yeoman generators
while maintaining full backward compatibility with existing generators.

**Changes:**

1. **Interface (`youi-events.ts`)**:
   - Added `doGeneratorProgress()` method for new progress system
   - Kept `doGeneratorInstall()` for backward compatibility (CRITICAL)
   - Changed `doGeneratorDone` return type from `void` to `Thenable<any>`
     to support async "Finalising..." phase

2. **Core Logic (`yeomanui.ts`)**:
   - Checks generator's `options.showGeneratorProgress` flag
   - If `true`: Enhanced 3-phase progress (writing, install, end)
   - If `false` or absent: Classic "Installing dependencies..." toast
   - Ensures zero breaking changes for existing generators

3. **VS Code Implementation (`vscode-youi-events.ts`)**:
   - `doGeneratorInstall()`: Shows classic toast (preserved)
   - `doGeneratorProgress()`: Shows enhanced progress with:
     * Project name in title: "Generating {projectName}"
     * Three phases: "Creating project files...", "Installing dependencies...", "Finalising..."
     * Minimum phase durations for visibility (2s, 0s, 1s)
     * Respects VS Code setting `ApplicationWizard.showGeneratorProgress`
   - `doGeneratorDone()`: Now async to show "Finalising..." before closing

4. **Messages (`messages.ts`)**:
   - Added localized progress messages following existing patterns:
     * `progress_preparing`, `progress_writing_files`
     * `progress_installing`, `progress_finalising`

5. **Test Fixes (`vscode-youi-events.spec.ts`)**:
   - Added `void` operator to async `doGeneratorDone()` calls
   - Fixes lint errors after return type change

6. **WebSocket Support (`server-youi-events.ts`)**:
   - Implemented `doGeneratorProgress()` for WebSocket clients
   - Changed `doGeneratorDone()` return type to `Promise<void>`

**Backward Compatibility:**
- Generators without `showGeneratorProgress` option: ✅ Classic toast
- Generators with `showGeneratorProgress: false`: ✅ Classic toast
- Generators with `showGeneratorProgress: true`: ✨ Enhanced progress

**Related:**
This addresses the revert of PR SAP#576 which broke backward compatibility.
- Added doGeneratorProgress() method to TestEvents mock classes
- Changed doGeneratorDone() return type from void to Thenable<any>
- Fixes TypeScript compilation errors after interface changes
… property

The showInstallMessage method now includes cancellable: false in the
withProgress options, so the test mock expectation must match.
…ications

This completes the backward-compatible implementation of enhanced progress
notifications for Yeoman generators, based on the reverted PR SAP#576.

Key Changes:
- yeomanui.ts: Always registers all three event handlers (writing, install, end)
- vscode-youi-events.ts: Falls back to classic toast if showProgress=false
- Enhanced progress shows project name with three phases
- All 289 tests passing

Backward Compatibility:
- No showGeneratorProgress option: Classic "Installing dependencies..." toast
- showGeneratorProgress: false: Classic toast
- showGeneratorProgress: true: Enhanced multi-phase progress

Locally tested with tools-suite Fiori generator.
Enhanced progress shows: "Creating project files..." → "Installing
dependencies..." → "Finalising..."

Addresses the backward compatibility issue that caused PR SAP#576 to be reverted.
Added 5 comprehensive tests to ensure backward compatibility:
- showProgress=false on install phase: Falls back to classic toast
- showProgress=false on writing/end phases: Does nothing (no notification)
- showProgress=true with project name: Shows enhanced progress
- showProgress=true without project name: Shows 'Generating...'

All 294 tests passing. Ensures zero breaking changes for generators
that don't opt in to enhanced progress notifications.
…e display

- Fix race condition where doGeneratorDone() async cleanup not awaited
  - Made onGeneratorSuccess() and onGeneratorFailure() async
  - Changed void doGeneratorDone() to await doGeneratorDone()
  - Added void prefix for event handler callbacks that can't await

- Fix shared state collision between multiple generators
  - Clear resolveFunc after calling it to prevent reuse

- Fix empty message body in backward compatibility mode
  - Detect empty string in showInstallMessage() and show progress_installing message

- Fix lint errors: added void prefix to test calls of async methods
- Fix enhanced mode flag not set if generator skips 'writing' phase
  - Remove phase === 'writing' check, allow any first phase to initialize
- Fix setTimeout race condition between generators
  - Capture reporter instance, only update if still the same instance
- Fix state not reset between generator runs
  - Reset all progress state when notification closes
  - Clear: isEnhancedProgressMode, currentPhase, phaseStartTime, currentProjectName
- Add tests for type="module" and type="" paths in doGeneratorDone
- Add tests for enhanced mode "Finalising..." message display
- Add tests for skipResolve parameter in showDoneMessage
- Add tests for phase transition logic (setTimeout and immediate paths)
- Add tests for backward compatibility with all phases (writing, install, end)
- Add tests for starting with different phases in enhanced mode
- Add test for withProgress callback execution to cover lines 302-319
- Fix TypeScript error: add explicit void return type to arrow function
- Fix ESLint errors: remove unused variables and async without await

Coverage increased from 89.64% to 92.04% (lines)
vscode-youi-events.ts coverage: 99.17% lines, 98.91% branches
All vscode-youi-events tests passing: 53 passing

@ilya-taupeka ilya-taupeka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks good, but there are the following concerns.

Comment thread projects/yeoman-ui/packages/backend/src/webSocketServer/server-youi-events.ts Outdated
Comment thread projects/yeoman-ui/packages/backend/src/vscode-youi-events.ts
Comment thread projects/yeoman-ui/packages/backend/src/vscode-youi-events.ts
…ess notifications

Fixes all 4 review concerns from PR SAP#613:

1. WebSocket legacy install fallback
   - Added fallback to doGeneratorInstall() when showProgress=false and phase=install
   - Preserves backward compatibility for non-opted-in WebSocket generators

2. Frontend generatorProgress RPC handler
   - Added generatorProgress(projectName, phase) method to App.vue
   - Registered in initRpc() functions list
   - Handles multi-phase progress updates in WebSocket mode

3. False "manual close" telemetry
   - Set GENERATOR_COMPLETED=true before doClose() in enhanced progress mode
   - Prevents AbstractWebviewPanel from treating webview disposal as manual close

4. Race condition in delayed phase transitions
   - Added pendingPhaseTimer to track and cancel pending setTimeout callbacks
   - Clear timer before scheduling new phase transitions
   - Clear timer in doGeneratorDone() and showInstallMessage() cleanup
   - Prevents out-of-order phase messages (e.g., "Finalising" -> "Installing" -> "Finalising")

All changes maintain backward compatibility - generators without showGeneratorProgress
option continue to show classic "Installing dependencies..." toast.

Tested with yeoman-ui 1.27.2 + application-modeler 1.33.0 + @sap/generator-fiori 1.33.0
Multi-phase progress confirmed working: "Writing files" → "Installing" → "Finalising"

@korotkovao korotkovao left a comment

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.

Thank you @ilya-taupeka I have addressed you feedback and replied to the comments.

Adds comprehensive test coverage for the review fixes:

- Timer cancellation on phase transitions (doGeneratorProgress)
- Timer cleanup in doGeneratorDone
- Timer cleanup in showInstallMessage cleanup
- WebSocket backward compatibility (server-youi-events)
- Enhanced mode phase transitions with minimum durations

Coverage now: 92.14% (was 91.79%), exceeds 92% threshold
Tests: 325 passing

Related to PR SAP#613 review feedback
TestRpc needs to extend RpcCommon abstract class and provide the
required baseLogger via constructor. Changed from 'implements' to
'extends' and removed duplicate method implementations.

Fixes TypeScript compilation error:
  Property 'baseLogger' is missing in type 'TestRpc'
Adds test coverage for the new generatorProgress method in App.vue:
- Writing phase updates
- Install phase updates
- End phase updates
- Handling undefined project name

Coverage now: 97.05% functions (was 95.58%), exceeds 96% threshold
Tests: 119 passing (was 115)
ilya-taupeka
ilya-taupeka previously approved these changes Sep 23, 2026

@ilya-taupeka ilya-taupeka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!
Thank you @korotkovao for addressing the feedback.

Note:
Final PR #576 kept a runtime check for ApplicationWizard.showGeneratorProgress before entering enhanced progress mode. PR #613 currently only checks the generator-level showProgress option, so a consumer/user setting that disables enhanced progress would be ignored.

If that setting is still part of the expected contract, please restore the setting gate and fall back to the legacy install progress on the install phase when enhanced progress is disabled.

…ng gate

Restores the VS Code setting check from original PR SAP#576 that was
inadvertently removed in the review fixes.

The setting gate allows consumers (app-modeler, app-generator) to let
users opt out of enhanced progress notifications via VS Code settings.

When ApplicationWizard.showGeneratorProgress is false:
- Falls back to classic 'Installing dependencies...' toast on install phase
- Does nothing on writing/end phases (backward compatible behavior)

When the setting is true (default) AND generator opts in:
- Shows enhanced multi-phase progress notifications

Added tests:
- Setting disabled + showProgress=true falls back to classic
- Setting enabled + showProgress=true enters enhanced mode

Tests: 328 passing
…eclaration

Adds the setting declaration to yeoman-ui's package.json to make the
ApplicationWizard.showGeneratorProgress setting visible in VS Code Settings UI.

The setting controls whether to show enhanced multi-phase progress notifications
(Writing files → Installing → Finalising) or the classic 'Installing dependencies...'
toast during project generation.

- type: boolean
- default: true (enhanced mode)
- scope: resource

When disabled, falls back to classic behavior for users who prefer the simpler
single-toast experience.
Comment thread projects/yeoman-ui/packages/backend/package.json Outdated

@ilya-taupeka ilya-taupeka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@ilya-taupeka
ilya-taupeka merged commit f258d3f into SAP:main Sep 24, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants