Upgrade package inquirer from 11.1.0 to 12.3 #3549
Conversation
WalkthroughThis pull request upgrades the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/setup/askForCustomPort/askForCustomPort.spec.ts (1)
5-14: LGTM! Consider enhancing the documentation.The new mocking implementation correctly handles inquirer v12+ compatibility while preserving the actual module's functionality. The approach is robust and follows vitest best practices.
Consider expanding the comment to better document why this specific mocking approach is needed:
-// ✅ Fix Inquirer Mocking for v12+ +// ✅ Fix Inquirer Mocking for v12+ +// This approach preserves actual inquirer functionality while allowing prompt mocking. +// Required due to ESM changes in inquirer v12.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/setup/askForCustomPort/askForCustomPort.spec.ts(1 hunks)
🔇 Additional comments (1)
src/setup/askForCustomPort/askForCustomPort.spec.ts (1)
23-25: Verify prompt response format compatibility with inquirer v12.The AI summary indicates that inquirer v12 expects array-wrapped configuration objects, but the test mocks use direct object responses. Let's verify this is still compatible with the new version.
Also applies to: 32-34, 56-57, 65-70, 80-81, 89-91, 99-109
✅ Verification successful
🌐 Web query:
What is the expected response format for inquirer.prompt in version 12?💡 Result:
Inquirer.js v12's `inquirer.prompt` returns responses as **a Promise resolving to an object** where answers are keyed by each question's `name` property. Here's the detailed structure: ## Response Format - **Object keys** match the `name` field from your question definitions - **Values** contain user responses in appropriate data types: - Single-value inputs: `string`/`number`/`boolean` - Multi-select (checkbox): `array` - List selections: `string` (selected value) Example response for a pizza ordering prompt: ```javascript { pizzaType: 'Pepperoni', // from list selection size: 'Large', // from rawlist toppings: ['Cheese','Olives'],// from checkbox specialInstructions: 'Extra crispy' // from input } ``` ## Key Implementation Details 1. **Promise-based** (async/await compatible): ```javascript const answers = await inquirer.prompt(questions); ``` 2. **Nested objects** supported through dot-notation names: ```javascript // Question definition { name: 'address.street', ... } // Response structure { address: { street: '123 Main St' } } ``` 3. **Type preservation**: - `number` type questions return numeric values - `confirm` type returns boolean 4. **Error handling**: ```javascript try { const answers = await inquirer.prompt(...); } catch (error) { if (error.isTtyError) { /* Rendering error */ } else { /* Other errors */ } } ``` This format remains consistent across all prompt types (input, list, checkbox, etc.)[1][4][7]. The key evolution from older versions is the shift from callback-style to Promise-based responses[2][10]. Citations: - 1: https://www.npmjs.com/package/inquirer - 2: https://www.npmjs.com/package/inquirer/v/0.1.12 - 3: https://github.com/enquirer/enquirer - 4: https://www.educative.io/answers/how-to-use-the-inquirer-node-package - 5: https://python-inquirer.readthedocs.io/en/latest/ - 6: https://python-inquirer.readthedocs.io/_/downloads/en/latest/pdf/ - 7: https://javascript.plainenglish.io/how-to-inquirer-js-c10a4e05ef1f?gi=06ccc5b37523 - 8: https://github.com/SBoudrias/Inquirer.js/releases - 9: https://www.digitalocean.com/community/tutorials/nodejs-interactive-command-line-prompts - 10: https://geshan.com.np/blog/2023/03/inquirer-js/
Compatibility Verified for inquirer.prompt Response Format
The latest version of inquirer (v12) returns responses as a Promise that resolves to an object keyed by each question's name. This means the current test mock using a direct object (as shown on lines 23‑25 and elsewhere) is valid and compatible with inquirer v12.
…ation/talawa-admin into upgrade_inquire
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(1 hunks)src/setup/askAndSetDockerOption/askAndSetDockerOption.spec.ts(1 hunks)src/setupTests.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/setup/askAndSetDockerOption/askAndSetDockerOption.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3549 +/- ##
====================================================
+ Coverage 85.69% 85.91% +0.21%
====================================================
Files 342 342
Lines 8881 8882 +1
Branches 1911 1911
====================================================
+ Hits 7611 7631 +20
+ Misses 930 906 -24
- Partials 340 345 +5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/setup/askForDocker/askForDocker.spec.ts (3)
5-14: Good approach for mocking inquirer v12+, consider adding type safety.The approach of importing the actual module and selectively overriding methods is robust and maintainable. However, consider these improvements:
vi.mock('inquirer', async () => { - const actual = await vi.importActual('inquirer'); + const actual = await vi.importActual<typeof import('inquirer')>('inquirer') + .catch(() => { + console.error('Failed to import inquirer'); + return {}; + }); return { default: { ...actual, prompt: vi.fn(), }, }; });
80-88: Consider making mocks more flexible and explicit.The mocks are well-structured, but could be improved for better testing flexibility:
vi.mock('../askForTalawaApiUrl/askForTalawaApiUrl', () => ({ askForTalawaApiUrl: vi .fn() - .mockResolvedValue('https://talawa-api.example.com'), + .mockResolvedValue(process.env.TEST_API_URL || 'https://talawa-api.example.com'), })); vi.mock('../updateEnvFile/updateEnvFile', () => ({ - default: vi.fn(), + default: vi.fn().mockResolvedValue(undefined), }));
90-106: Enhance test coverage with assertions and edge cases.While the tests cover the basic flows, consider these improvements:
describe('askAndUpdateTalawaApiUrl', () => { test('should proceed with API setup when user confirms', async () => { + const mockUpdateEnvFile = vi.fn().mockResolvedValue(undefined); + vi.mock('../updateEnvFile/updateEnvFile', () => ({ + default: mockUpdateEnvFile, + })); + vi.spyOn(inquirer, 'prompt') - .mockResolvedValueOnce({ shouldSetTalawaApiUrlResponse: true }) // ✅ Covers line 35 + .mockResolvedValueOnce({ shouldSetTalawaApiUrlResponse: true }) .mockResolvedValueOnce({ dockerAppPort: '4321' }); await expect(askAndUpdateTalawaApiUrl()).resolves.not.toThrow(); + expect(mockUpdateEnvFile).toHaveBeenCalled(); }); test('should skip API setup when user declines', async () => { + const mockUpdateEnvFile = vi.fn().mockResolvedValue(undefined); + vi.mock('../updateEnvFile/updateEnvFile', () => ({ + default: mockUpdateEnvFile, + })); + vi.spyOn(inquirer, 'prompt').mockResolvedValueOnce({ shouldSetTalawaApiUrlResponse: false, }); await expect(askAndUpdateTalawaApiUrl()).resolves.not.toThrow(); + expect(mockUpdateEnvFile).not.toHaveBeenCalled(); }); + + test('should handle errors from askForTalawaApiUrl', async () => { + vi.spyOn(inquirer, 'prompt') + .mockResolvedValueOnce({ shouldSetTalawaApiUrlResponse: true }); + + const error = new Error('API URL validation failed'); + vi.mock('../askForTalawaApiUrl/askForTalawaApiUrl', () => ({ + askForTalawaApiUrl: vi.fn().mockRejectedValue(error), + })); + + await expect(askAndUpdateTalawaApiUrl()).rejects.toThrow(error); + }); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/setup/askForDocker/askForDocker.spec.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
palisadoes
merged commit 232faf3
into
PalisadoesFoundation:develop-postgres
Upgraing package inquirer from 11.1 tot 12.4
Issue: #3517
Summary by CodeRabbit