Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the ext:info command to include emitted events in its output, accompanied by comprehensive unit tests and a changelog entry. The feedback suggests two improvements: explicitly verifying that spec.events is an array using Array.isArray() to prevent runtime errors, and refactoring the event string construction to use a single conditional template literal instead of string concatenation for better readability.
| lines.push(`* ${resource.name} (${resource.type})`); | ||
| } | ||
| } | ||
| if (spec.events && spec.events.length > 0) { |
There was a problem hiding this comment.
To prevent potential runtime errors or unexpected output (such as printing * undefined) if spec.events is malformed (e.g., parsed as a string or object instead of an array), we should explicitly verify that spec.events is an array using Array.isArray(), similar to how spec.params is checked on line 75.
| if (spec.events && spec.events.length > 0) { | |
| if (spec.events && Array.isArray(spec.events) && spec.events.length > 0) { |
References
- Use strict null checks and handle undefined/null explicitly. (link)
| if (spec.events && spec.events.length > 0) { | ||
| lines.push("", "**Events Emitted:**"); | ||
| for (const event of spec.events) { | ||
| lines.push(`* ${event.type}` + (event.description ? `: ${event.description}` : "")); |
There was a problem hiding this comment.
Using string concatenation (+) with template literals can be less readable. We can simplify this by using a single template literal with a conditional expression, or by splitting it into a cleaner conditional template literal.
| lines.push(`* ${event.type}` + (event.description ? `: ${event.description}` : "")); | |
| lines.push(event.description ? `* ${event.type}: ${event.description}` : `* ${event.type}`); |
Description
In
ext:info, extension specifications declaring custom Eventarc events underspec.eventswere omitted from both Markdown and terminal output formats. This caused automated README documentation generated viafirebase ext:info --markdownto lack emitted event documentation. This change formats and displays theEvents Emitted:section wheneverspec.eventsis present on the extension specification.Fixes #8209
Scenarios Tested
src/commands/ext-info.spec.tsverifyingEvents Emitted:header and event details are rendered for--markdownand terminal modes.spec.eventsis omitted or empty.npx mocha src/commands/ext-info.spec.ts(4 passing).npx mocha src/extensions/*.spec.ts(401 passing).npm run test:compile(clean exit).npx prettier --checkandnpx eslinton modified files.Sample Commands
firebase ext:info firestore-bigquery-export --markdownfirebase ext:info ./my-extension