Repository navigation
feat(yeoman-UI): backward compatible progress notifications - #613
ilya-taupeka merged 16 commits into
Conversation
…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
left a comment
There was a problem hiding this comment.
It looks good, but there are the following concerns.
…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
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
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.
…etting declaration" This reverts commit c673820.
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 generatorsshowGeneratorProgress: trueoption to enable enhanced progressBackward Compatibility
doGeneratorInstall()preserved: Classic behavior unchangedshowProgress: false- generators without opt-in get classic single-phase toastBug Fixes (from code review)
Testing
Breaking Changes
None - fully backward compatible.
Related