feat: Support text template groups #494
Conversation
|
WalkthroughThis pull request implements several modifications across multiple packages. It updates and refines template references and parameters in generator modules, introduces validation for unique file IDs, and changes naming conventions (e.g., switching from PascalCase to camelCase). A new metadata deletion function is added and integrated into the synchronization process. Additionally, the text template rendering process is streamlined by replacing single file actions with grouped actions, and a new utility for computing common path prefixes is introduced along with corresponding unit tests. Changes
Sequence Diagram(s)sequenceDiagram
participant GFD as generateForDirectory
participant TM as TemplateMetadataWriter
participant DM as deleteMetadataFiles
participant WM as WriteMetadata
GFD->>TM: Check if metadata writing is enabled
alt Enabled
GFD->>DM: Invoke deleteMetadataFiles(projectDirectory)
DM->>DM: Recursively delete metadata files
DM-->>GFD: Deletion complete
end
GFD->>WM: Write new metadata files
sequenceDiagram
participant Caller as Generator
participant RTG as renderTextTemplateGroupAction
participant RT as renderTextTemplateFileAction
Caller->>RTG: Invoke with text template group and variables
RTG->>RT: For each template in group, initiate rendering
RT-->>RTG: Return rendered file output
RTG-->>Caller: Consolidate and return group output
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
packages/utils/src/paths/get-common-path-prefix.ts (2)
3-10: Documentation could be more detailed.The JSDoc is good but could be enhanced with examples and edge case descriptions to improve developer understanding.
/** * Returns the highest common ancestor path from a list of absolute paths. * * Only works for POSIX-style paths. + * + * @example + * getCommonPathPrefix(['/home/user/a.txt', '/home/user/b.txt']) // returns '/home/user' + * getCommonPathPrefix([]) // returns '' + * getCommonPathPrefix(['/home/user/file.txt']) // returns '/home/user' * * @param paths - Array of absolute paths * @returns Common ancestor path */
32-35: Return statement could be simplified for better readability.The current condition checking is a bit complex and could be made more readable.
- return commonParts.length > 0 && - !(commonParts.length === 1 && commonParts[0] === '') - ? commonParts.join(path.sep) - : path.parse(first).root; + // Check for root path case (empty array or just empty string) + if (commonParts.length === 0 || (commonParts.length === 1 && commonParts[0] === '')) { + return path.parse(first).root; + } + return commonParts.join(path.sep);packages/utils/src/paths/get-common-path-prefix.unit.test.ts (1)
5-47: Consider adding tests for invalid inputs.While the current tests are comprehensive for valid inputs, consider adding tests for non-absolute paths or other edge cases.
it('should handle non-absolute paths gracefully', () => { // This might require modifying the function to validate input paths const paths = ['relative/path/file.txt', '/absolute/path/file.txt']; // Define expected behavior for this case expect(() => getCommonPathPrefix(paths)).toThrow(/absolute paths/i); // Or if you want to handle it silently: // expect(getCommonPathPrefix(paths)).toBe(''); });packages/sync/src/templates/text-template/render-text-template-group-action.unit.test.ts (1)
16-16: Fix typo in the describe block titleThere's a duplication of the word "Group" in the describe function name.
-describe('renderTextTemplateGroupGroupAction', () => { +describe('renderTextTemplateGroupAction', () => {packages/sync/src/templates/text-template/text-template-file-extractor.unit.test.ts (1)
67-185: Test coverage for grouped text templates looks comprehensive.
Defining separate text files within multiple groups and then verifying the resulting grouped output is properly structured. This test ensures the grouping logic is enforced and yields the expected code generation. It might be insightful to add a small negative/edge scenario (e.g., empty group name or missing group property) to verify error handling or fallback behavior when grouping is incomplete.packages/sync/src/templates/metadata/delete-metadata.ts (2)
34-37: Consider adding error handling for template metadata file deletion.Error handling is implemented for the generator metadata file deletion but not for template metadata files. Consider adding a similar try/catch block to handle potential errors when deleting template metadata files.
- await fs.unlink(fullPath); + try { + await fs.unlink(fullPath); + } catch { + // Ignore if file doesn't exist or can't be deleted + }
27-38: Consider using Promise.all for parallel processing.For directories with many files and subdirectories, processing entries sequentially could be slow. Consider using Promise.all to process entries in parallel for better performance:
- // Process each entry - for (const entry of entries) { - const fullPath = path.join(directory, entry.name); - - if (entry.isDirectory()) { - // Recursively process subdirectories - await deleteMetadataFiles(fullPath); - } else if (entry.name === TEMPLATE_METADATA_FILENAME) { - // Delete template metadata file - await fs.unlink(fullPath); - } - } + // Process all entries in parallel + await Promise.all( + entries.map(async (entry) => { + const fullPath = path.join(directory, entry.name); + + if (entry.isDirectory()) { + // Recursively process subdirectories + await deleteMetadataFiles(fullPath); + } else if (entry.name === TEMPLATE_METADATA_FILENAME) { + // Delete template metadata file + try { + await fs.unlink(fullPath); + } catch { + // Ignore if file doesn't exist or can't be deleted + } + } + }) + );packages/sync/src/templates/text-template/render-text-template-group-action.ts (1)
52-64: Consider parallel processing for template rendering.For groups with many templates, sequential processing could be slow. Consider using Promise.all to render templates in parallel for better performance:
- execute: async (builder) => { - for (const [key, template] of Object.entries(group.templates)) { - const destination = path.join(baseDirectory, template.destination); - await builder.apply( - renderTextTemplateFileAction({ - template: template.template, - destination, - variables: (variables as Record<string, never>)[key], - options: options?.[key], - }), - ); - } - }, + execute: async (builder) => { + await Promise.all( + Object.entries(group.templates).map(async ([key, template]) => { + const destination = path.join(baseDirectory, template.destination); + try { + await builder.apply( + renderTextTemplateFileAction({ + template: template.template, + destination, + variables: variables && typeof variables === 'object' ? variables[key as keyof typeof variables] : undefined, + options: options?.[key], + }), + ); + } catch (error) { + throw new Error(`Failed to render template "${key}": ${error instanceof Error ? error.message : String(error)}`); + } + }) + ); + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
⛔ Files ignored due to path filters (4)
packages/fastify-generators/src/generators/core/fastify-scripts/generated/raw-templates.tsis excluded by!**/generated/**packages/fastify-generators/src/generators/core/readme/generated/text-templates.tsis excluded by!**/generated/**packages/react-generators/src/generators/core/react/generated/raw-templates.tsis excluded by!**/generated/**packages/react-generators/src/generators/core/react/generated/text-templates.tsis excluded by!**/generated/**
📒 Files selected for processing (26)
packages/fastify-generators/src/generators/core/fastify-scripts/index.ts(1 hunks)packages/fastify-generators/src/generators/core/readme/index.ts(1 hunks)packages/project-builder-server/src/sync/index.ts(2 hunks)packages/react-generators/src/generators/core/react/index.ts(2 hunks)packages/sync/src/output/prepare-generator-files/prepare-generator-files.ts(1 hunks)packages/sync/src/output/prepare-generator-files/prepare-generator-files.unit.test.ts(2 hunks)packages/sync/src/templates/extractor/run-template-file-extractors.ts(1 hunks)packages/sync/src/templates/metadata/delete-metadata.ts(1 hunks)packages/sync/src/templates/metadata/delete-metadata.unit.test.ts(1 hunks)packages/sync/src/templates/metadata/index.ts(1 hunks)packages/sync/src/templates/raw-template/raw-template-file-extractor.ts(2 hunks)packages/sync/src/templates/raw-template/raw-template-file-extractor.unit.test.ts(1 hunks)packages/sync/src/templates/raw-template/render-raw-template-action.ts(2 hunks)packages/sync/src/templates/text-template/index.ts(1 hunks)packages/sync/src/templates/text-template/render-text-template-file-action.ts(2 hunks)packages/sync/src/templates/text-template/render-text-template-file-action.unit.test.ts(1 hunks)packages/sync/src/templates/text-template/render-text-template-group-action.ts(1 hunks)packages/sync/src/templates/text-template/render-text-template-group-action.unit.test.ts(1 hunks)packages/sync/src/templates/text-template/text-template-file-extractor.ts(6 hunks)packages/sync/src/templates/text-template/text-template-file-extractor.unit.test.ts(1 hunks)packages/sync/src/templates/text-template/types.ts(2 hunks)packages/sync/src/templates/types.ts(1 hunks)packages/utils/src/node.ts(1 hunks)packages/utils/src/paths/get-common-path-prefix.ts(1 hunks)packages/utils/src/paths/get-common-path-prefix.unit.test.ts(1 hunks)packages/utils/src/paths/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (13)
packages/utils/src/paths/get-common-path-prefix.unit.test.ts (1)
packages/utils/src/paths/get-common-path-prefix.ts (1)
getCommonPathPrefix(11-36)
packages/project-builder-server/src/sync/index.ts (1)
packages/sync/src/templates/metadata/delete-metadata.ts (1)
deleteMetadataFiles(16-39)
packages/fastify-generators/src/generators/core/fastify-scripts/index.ts (1)
packages/fastify-generators/src/generators/core/fastify-scripts/generated/raw-templates.ts (1)
CORE_FASTIFY_SCRIPTS_RAW_TEMPLATES(8-10)
packages/fastify-generators/src/generators/core/readme/index.ts (1)
packages/fastify-generators/src/generators/core/readme/generated/text-templates.ts (1)
CORE_README_TEXT_TEMPLATES(9-11)
packages/sync/src/templates/raw-template/raw-template-file-extractor.ts (1)
packages/sync/src/templates/raw-template/types.ts (1)
RawTemplateFile(24-24)
packages/sync/src/output/prepare-generator-files/prepare-generator-files.unit.test.ts (1)
packages/sync/src/output/generator-task-output.ts (1)
FileData(49-62)
packages/sync/src/templates/metadata/delete-metadata.ts (1)
packages/sync/src/templates/constants.ts (2)
GENERATOR_INFO_FILENAME(1-1)TEMPLATE_METADATA_FILENAME(2-2)
packages/sync/src/templates/text-template/render-text-template-file-action.ts (2)
packages/core-generators/src/renderers/typescript/ts-code-utils.ts (1)
template(161-182)packages/sync/src/templates/text-template/types.ts (1)
TEXT_TEMPLATE_TYPE(7-7)
packages/sync/src/templates/text-template/render-text-template-group-action.ts (4)
packages/sync/src/templates/text-template/types.ts (3)
TextTemplateFile(47-57)InferTextTemplateVariablesFromTemplate(65-68)TextTemplateGroup(78-88)packages/sync/src/output/generator-task-output.ts (1)
WriteFileOptions(22-44)packages/sync/src/output/builder-action.ts (1)
BuilderAction(6-8)packages/sync/src/templates/text-template/render-text-template-file-action.ts (1)
renderTextTemplateFileAction(30-130)
packages/react-generators/src/generators/core/react/index.ts (3)
packages/react-generators/src/generators/core/react/generated/raw-templates.ts (1)
CORE_REACT_RAW_TEMPLATES(8-10)packages/sync/src/templates/text-template/render-text-template-group-action.ts (1)
renderTextTemplateGroupAction(43-66)packages/react-generators/src/generators/core/react/generated/text-templates.ts (1)
CORE_REACT_TEXT_TEMPLATES(47-49)
packages/sync/src/templates/text-template/text-template-file-extractor.ts (4)
packages/sync/src/templates/text-template/types.ts (1)
TextTemplateFile(47-57)packages/sync/src/templates/extractor/template-file-extractor.ts (1)
TemplateFileExtractorFile(17-28)packages/utils/src/paths/get-common-path-prefix.ts (1)
getCommonPathPrefix(11-36)packages/utils/src/maps/group-by.ts (1)
mapGroupBy(12-27)
packages/sync/src/templates/text-template/text-template-file-extractor.unit.test.ts (3)
packages/sync/src/templates/extractor/template-file-extractor-test-utils.ts (1)
TemplateFileExtractorTestUtils(28-47)packages/sync/src/templates/text-template/text-template-file-extractor.ts (1)
TextTemplateFileExtractor(22-168)packages/sync/src/templates/text-template/types.ts (1)
TEXT_TEMPLATE_TYPE(7-7)
packages/sync/src/templates/text-template/render-text-template-group-action.unit.test.ts (3)
packages/sync/src/templates/text-template/types.ts (2)
createTextTemplateGroup(90-94)createTextTemplateFile(59-63)packages/sync/src/templates/text-template/render-text-template-group-action.ts (1)
renderTextTemplateGroupAction(43-66)packages/sync/src/output/builder-action-test-helpers.ts (1)
testAction(35-42)
🔇 Additional comments (53)
packages/utils/src/paths/get-common-path-prefix.ts (2)
11-13: LGTM! Edge cases are properly handled.The function correctly handles empty arrays and single-path scenarios.
20-30: The algorithm is well-implemented.The implementation effectively finds the common path prefix by comparing path segments. The early break when no common parts are found is a good optimization.
packages/utils/src/paths/index.ts (1)
1-1: LGTM! Proper module export.Follows standard pattern for re-exporting module contents through an index file.
packages/utils/src/node.ts (1)
4-4: LGTM! Consistent export pattern.This export follows the same pattern as the existing exports, maintaining consistency.
packages/utils/src/paths/get-common-path-prefix.unit.test.ts (6)
5-9: Tests cover basic edge cases.Good test coverage for the empty array case.
10-14: LGTM! Single path case handled correctly.The test correctly verifies that the function returns the directory of a single path.
16-23: LGTM! Test for common use case is thorough.This test effectively validates the primary function of finding a common path prefix.
25-28: Test for paths with minimal common prefix.This test correctly verifies behavior when paths share only a minimal common prefix.
30-37: LGTM! Test handles varying path depths well.This test correctly verifies that the function works with paths of different directory depths.
39-46: LGTM! Root directory case is covered.This test ensures the function works correctly when only the root directory is common.
packages/sync/src/templates/types.ts (1)
30-33: Well-implemented addition of the template group property.Adding the optional
groupproperty to theTemplateFileBaseinterface is a clean, backward-compatible change that supports the new text template groups feature. The property is properly typed and well-documented with JSDoc comments.packages/sync/src/templates/metadata/index.ts (1)
1-1: Appropriate export of the new metadata deletion functionality.The addition of this export statement makes the metadata deletion functionality available to consumers of this module, which aligns with the PR objective of implementing text template groups support. The export follows the same pattern as the existing exports.
packages/fastify-generators/src/generators/core/fastify-scripts/index.ts (1)
65-65: Good adherence to standardized naming conventions.The change from
TsconfigRawTemplatetotsconfigimproves consistency by aligning with the camelCase naming convention used in theCORE_FASTIFY_SCRIPTS_RAW_TEMPLATESobject defined in the raw-templates.ts file.packages/sync/src/templates/extractor/run-template-file-extractors.ts (1)
97-97: Good normalization of file paths for cross-platform compatibility.Replacing the system-specific path separators with POSIX-style separators (
/) ensures consistent path formatting across different operating systems. This is particularly important for template processing where paths might be used for identification, comparison, or storage rather than direct file system access.Just ensure that any code consuming this path property is aware that it uses normalized POSIX-style separators if it needs to directly interact with the file system.
packages/sync/src/templates/text-template/index.ts (1)
1-2: Updated exports to support text template groups.The export statements have been updated to reflect a significant restructuring of the text template functionality. The code now exports from two specialized files instead of a single file, aligning with the PR's objective to support text template groups.
This change enhances modularity by separating the file action and group action concerns into distinct files, which is a good architectural decision. The new file structure better communicates the expanded capabilities for handling both individual templates and template groups.
packages/sync/src/templates/text-template/render-text-template-file-action.unit.test.ts (1)
6-6: Updated import path to match new file structure.The import statement has been updated to reflect the renaming of the source file from
render-text-template-action.jstorender-text-template-file-action.js, which is consistent with the restructuring seen in the index.ts file.This change ensures tests continue to work correctly after the codebase restructuring to support template groups.
packages/project-builder-server/src/sync/index.ts (2)
10-10: Added import for new metadata deletion functionality.The import statement now includes the
deleteMetadataFilesfunction from the sync package, which is used to recursively delete metadata files from a specified directory.
169-169: Added metadata cleanup step before writing new metadata.This line adds a crucial cleanup step before writing new generator and template metadata, ensuring that outdated or conflicting metadata files are removed first. This prevents potential conflicts and provides a clean state for the new metadata.
The placement of this call is appropriate - it happens only when template metadata writing is enabled and immediately before the new metadata is written, maintaining a logical flow in the code.
packages/fastify-generators/src/generators/core/readme/index.ts (1)
31-39: Updated template reference and simplified rendering options.The template reference has been changed from a PascalCase format to camelCase format, and the
idparameter andshouldFormatoption have been removed, simplifying the rendering configuration.These changes align with the broader refactoring effort for template handling in the codebase. The
idparameter is likely now optional in the updatedrenderTextTemplateFileActionfunction, and formatting behavior may be controlled differently in the new implementation.The updated code is more concise while maintaining the same functionality.
packages/sync/src/output/prepare-generator-files/prepare-generator-files.ts (2)
82-97: Good implementation of file ID validation!This is an important validation to prevent ID conflicts which could lead to unexpected behavior with file tracking. The error message is clear and includes both the conflicting ID and the path of the previously registered file with that ID.
101-101: Clean use of the already constructed mapGood refactoring to use the constructed
fileIdToRelativePathMapdirectly instead of rebuilding it in the return statement.packages/sync/src/output/prepare-generator-files/prepare-generator-files.unit.test.ts (3)
27-27: Good implementation of unique ID generatorThe counter variable is a clean way to generate unique IDs for testing purposes.
31-31: Well-structured file ID generationUsing the counter to generate unique IDs for each test file is a good approach. This matches well with the new validation in the implementation file.
40-45: Good test case updateThis updated test case now explicitly tests the unique ID validation by providing a specific ID rather than relying on the counter. This provides better test coverage for the new validation logic.
packages/sync/src/templates/raw-template/raw-template-file-extractor.ts (4)
2-2: Good convention update from PascalCase to camelCaseThis change aligns the naming convention with JavaScript/TypeScript best practices for variable naming.
25-25: Consistent naming convention appliedGood update to use camelCase for template names, which is more appropriate for variable naming in JavaScript.
28-35: Simplified template string constructionThe code is now cleaner by directly using the
templateNamevariable in the template string construction.
36-36: Consistent export namingUpdated exports array to use the camelCase variable name, maintaining consistency.
packages/sync/src/templates/text-template/render-text-template-file-action.ts (4)
18-18: Good improvement to make ID optionalMaking the ID optional provides more flexibility and allows for using the template name as a default identifier.
109-109: Support for template groupingAdding the group property to the metadata allows templates to be organized into logical groups, which supports the new text template group feature mentioned in the PR objectives.
111-117: Improved conditional handling of variablesThe refined condition for setting template variables ensures that metadata is only created when there are actual variables to include, which is cleaner and more efficient.
122-122: Clean default ID implementationUsing the nullish coalescing operator provides a clean way to default to the template name when no ID is provided. This supports the group template functionality by allowing template IDs to be automatically derived from names when not explicitly provided.
packages/react-generators/src/generators/core/react/index.ts (3)
23-23: Good update to import the new template group rendering function.The import statement correctly includes the new
renderTextTemplateGroupActionfunction, which is consistent with the changes that follow.
141-141: Consistent naming convention for favicon template.The change from
FaviconRawTemplatetofaviconreflects a more consistent naming approach, likely part of a standardization effort across templates.
147-159: Excellent refactoring to use template groups.The replacement of multiple
renderTextTemplateFileActioncalls with a singlerenderTextTemplateGroupActioncall significantly improves the code:
- Reduces code duplication and simplifies maintenance
- Organizes template variables into a logical nested structure
- Makes the relationship between templates and their variables more explicit
- Leverages the new template group feature for better code organization
This change aligns perfectly with the PR objective of supporting text template groups.
packages/sync/src/templates/raw-template/raw-template-file-extractor.unit.test.ts (1)
63-63: Variable naming convention change looks goodThe changes from PascalCase to camelCase for template names (
TestRawTemplate→testandTest2RawTemplate→test2) are consistent with the PR objective of changing naming conventions. These changes have been applied consistently across variable declarations and the export object.Also applies to: 68-68, 74-75
packages/sync/src/templates/metadata/delete-metadata.unit.test.ts (1)
1-65: Well-structured tests for the metadata deletion functionalityThe test suite is well organized with appropriate setup and assertions. I appreciate the thorough testing of both successful deletion scenarios and graceful handling of non-existent files.
packages/sync/src/templates/text-template/render-text-template-group-action.unit.test.ts (1)
17-80: Well-implemented test for template group renderingThe test effectively demonstrates the functionality of rendering multiple templates in a group with variables. The setup is clear, and the assertions verify both the file count and the content of each rendered file.
packages/sync/src/templates/text-template/types.ts (3)
12-15: Good addition of group capability to templatesAdding the optional group property to the template metadata schema is a clean way to implement the template grouping feature. The JSDoc comment clearly explains its purpose.
19-27: Making variables optional is a good improvementMaking the variables property optional allows for templates without variables, which increases flexibility. The type safety is maintained while making the API more accommodating.
70-94: Well-designed interfaces for template groupsThe
TextTemplateGroupEntryandTextTemplateGroupinterfaces are well-defined with clear purposes. ThecreateTextTemplateGrouphelper function maintains type safety similar to the existingcreateTextTemplateFilefunction, providing a consistent API design.packages/sync/src/templates/text-template/text-template-file-extractor.unit.test.ts (1)
54-64: Good use of inline snapshot for validating code generation.
The inline snapshot captures the generated TypeScript content accurately, making it clear how the final output should look. This approach helps ensure the generated code’s structure remains stable.packages/sync/src/templates/text-template/text-template-file-extractor.ts (9)
2-4: Imports are well-organized.
Using specific imports (getCommonPathPrefix,camelCase,constantCase, etc.) clarifies purpose and makes code more readable and maintainable.
16-20: New interface elegantly centralizes code entries.
DefiningTypescriptCodeEntryhelps keep the structure of generated code blocks, exports, and imports consistent across methods.
30-30: Method return type clarifies usage of both code data and original path.
Returning a union ofTypescriptCodeEntry & { originalPath: string }makes it explicit that we need the file’s original location for additional processing (e.g., calculating relative paths).
37-37: Optional chaining on variables is a safe fallback.
Usingmetadata.variables ?? {}ensures that the loop executes safely even ifvariablesis undefined.
51-51: Switch to camelCase for template names is consistent with typical JavaScript conventions.
This change helps maintain a standard naming scheme for generated identifiers.
54-69: Refined structure for the returned code block.
The new approach provides clarity on how a single text template’s code block, exports, and imports are assembled. Separating the logic into discrete artifacts improves maintainability and debugging.
72-109: Group extraction logic is well-structured.
Splitting outextractTemplateFilesForGroupis a clean solution for bundling templates that share a group, and usinggetCommonPathPrefixhelps unify paths. This modular approach keeps single-file extraction logic separate from the grouping layer, enhancing readability.
118-131: Efficient concurrency for processing grouped vs. ungrouped files.
UsingpLimitensures large batches of files do not overwhelm resources. The approach to handle grouped files first and ungrouped files next is logical and avoids duplication.
146-151: Consolidated imports and exports generation is straightforward.
Merging imports viauniqand flattening combined exports ensures the finaltext-templates.tsis both minimal and consistent. This is a good approach for preventing name collisions and import redundancies.packages/sync/src/templates/raw-template/render-raw-template-action.ts (2)
13-13: Implementation of optional ID parameter looks good.Making the
idparameter optional in the interface improves API flexibility while maintaining backward compatibility.
42-42: Good implementation of fallback value for ID.Nice use of the nullish coalescing operator to provide a sensible default value when ID is not provided.
packages/sync/src/templates/text-template/render-text-template-group-action.ts
Outdated
Show resolved
Hide resolved
packages/sync/src/templates/text-template/render-text-template-group-action.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/sync/src/templates/text-template/render-text-template-group-action.ts (1)
67-71: Consider preserving the original error stack.Throwing a new error message improves clarity, but it also discards the original stack trace, which may hinder debugging. Including the stack trace (when available) can provide deeper insight into the cause of failure.
Below is an optional revision:
- throw new Error( - `Failed to render template "${key}": ${String(error)}` - ); + if (error instanceof Error) { + throw new Error(`Failed to render template "${key}": ${error.message}\n${error.stack}`); + } else { + throw new Error(`Failed to render template "${key}": ${String(error)}`); + }packages/sync/src/templates/text-template/render-text-template-group-action.unit.test.ts (1)
16-81: Add a negative test scenario to increase coverage.Currently, the test suite only covers the happy path where valid variables are provided. Testing for missing or malformed variables (e.g., omitting
TPL_NAME) would help confirm that errors are thrown or handled correctly, improving reliability.You could add a new test, for instance:
it('should throw an error when required variables are missing', async () => { const incompleteGroup = createTextTemplateGroup({ templates: { greeting: { destination: 'output/greeting.txt', template: createTextTemplateFile({ name: 'greeting', source: { path: 'greeting.txt' }, variables: { TPL_NAME: { description: 'The name to greet' }, }, }), }, }, }); const action = renderTextTemplateGroupAction({ group: incompleteGroup, baseDirectory: 'src', // Intentionally omit the 'greeting' variables }); await expect( testAction(action, { generatorInfo: { name: 'test-generator', baseDirectory: '/root/pkg/test-generator', }, }) ).rejects.toThrowError(/TPL_NAME/); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (2)
packages/sync/src/templates/text-template/render-text-template-group-action.ts(1 hunks)packages/sync/src/templates/text-template/render-text-template-group-action.unit.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/sync/src/templates/text-template/render-text-template-group-action.ts (4)
packages/sync/src/templates/text-template/types.ts (3)
TextTemplateFile(47-57)InferTextTemplateVariablesFromTemplate(65-68)TextTemplateGroup(78-88)packages/sync/src/output/generator-task-output.ts (1)
WriteFileOptions(22-44)packages/sync/src/output/builder-action.ts (1)
BuilderAction(6-8)packages/sync/src/templates/text-template/render-text-template-file-action.ts (1)
renderTextTemplateFileAction(30-130)
🔇 Additional comments (1)
packages/sync/src/templates/text-template/render-text-template-group-action.ts (1)
57-65: Good job adopting the safer variable access pattern.Your approach of checking that
variablesis an object before accessing properties helps prevent runtime errors whenvariablesmight be undefined or of a different type.
kingston
deleted the
kingston/eng-615-support-template-file-groups-to-enable-easier-usage-of
branch
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests