feat: Implement phase 1 of reverse template generator v2 #570
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change introduces the foundational infrastructure for Template Extractor V2, including new configuration schemas, plugin and runner systems, file container management, and utilities for reading, grouping, and updating template metadata. It also adds new extractors for raw and text templates, path root registration, and updates generators and CLI commands to integrate with the new extraction flow. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / Server
participant V2Runner as runTemplateExtractorsForProjectV2
participant Config as TemplateExtractorConfigLookup
participant Plugins as initializeTemplateExtractorPlugins
participant Extractors as TemplateFileExtractors
participant FileContainer as TemplateExtractorFileContainer
CLI->>V2Runner: runTemplateExtractorsForProjectV2(directory, app, context, logger, options)
V2Runner->>Config: initialize() and build generator package map
V2Runner->>Extractors: runTemplateFileExtractors(extractors, outputDirectory, generatorPackageMap, logger, options)
Extractors->>Config: readTemplateMetadataFiles(outputDirectory)
Extractors->>Plugins: initializeTemplateExtractorPlugins({ templateExtractors, context })
Extractors->>Extractors: groupTemplateFilesByType()
loop For each template type
Extractors->>Extractors: extractTemplateMetadataEntries()
end
Extractors->>Plugins: afterExtract hooks
Extractors->>Config: updateExtractorTemplateEntries()
loop For each template type
Extractors->>Extractors: writeTemplateFiles()
Extractors->>Extractors: writeGeneratedFiles()
end
Extractors->>Plugins: afterWrite hooks
Extractors->>FileContainer: commit()
V2Runner->>CLI: Extraction complete
Possibly related PRs
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
🦋 Changeset detectedLatest commit: 3d4bf73 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
🧹 Nitpick comments (41)
packages/fastify-generators/src/generators/core/readme/extractor.json (1)
10-13: Consider making the default project name more generic.The default value "todoer backend" appears specific to one project. Consider using a more generic placeholder like "My Project" or making it a required variable without a default value to ensure developers provide appropriate project names.
- "TPL_PROJECT": { - "description": "Name of the project", - "value": "todoer backend" - } + "TPL_PROJECT": { + "description": "Name of the project", + "value": "My Project" + }packages/sync/src/templates/extractor-v2/configs/try-create-extractor-json.unit.test.ts (1)
14-122: Consider refactoring test setup to reduce duplication.The test setup for
packagePath,generatorName, andpackageMapis repeated across multiple tests. Consider extracting common setup to improve maintainability.+const TEST_PACKAGE_PATH = '/test-package'; +const createTestPackageMap = () => new Map<string, string>([ + ['test-package', TEST_PACKAGE_PATH], +]); describe('tryCreateExtractorJson', () => { beforeEach(() => { vol.reset(); }); it('should create extractor.json when single generator file exists', async () => { // Arrange - const packagePath = '/test-package'; const generatorName = 'test-package#auth/login'; - const packageMap = new Map<string, string>([ - ['test-package', '/test-package'], - ]); + const packageMap = createTestPackageMap(); - vol.fromJSON({ - [`${packagePath}/src/generators/auth/login/login.generator.ts`]: - 'export const generator = {};', - }); + vol.fromJSON({ + [`${TEST_PACKAGE_PATH}/src/generators/auth/login/login.generator.ts`]: + 'export const generator = {};', + });packages/core-generators/src/renderers/text/utils.unit.test.ts (1)
5-23: Consider adding edge case tests for improved robustness.The current tests are solid but could benefit from additional edge cases to ensure comprehensive coverage:
+ describe('edge cases', () => { + it('should handle empty strings and special regex characters', () => { + const regexSpecial = getTextTemplateVariableRegExp('test$var'); + expect('test$var').toMatch(regexSpecial); + expect('test$var123').not.toMatch(regexSpecial); + }); + + it('should handle variables at string boundaries', () => { + const regex = getTextTemplateVariableRegExp('var'); + expect('var').toMatch(regex); + expect(' var ').toMatch(regex); + expect('var ').toMatch(regex); + expect(' var').toMatch(regex); + }); + });packages/sync/src/templates/extractor-v2/configs/providers-config.schema.ts (2)
14-14: Fix typo in JSDoc comment."inclues" should be "includes".
- * "type": "path", + * "type": "path",Actually, let me correct that - the typo is in line 14:
- * @param packagePathSpecifier - The package path specifier is a path that inclues + * @param packagePathSpecifier - The package path specifier is a path that includesWait, I'm looking at the wrong line. Let me check line 14 in this file... Line 14 has "name": "feature-root", which is correct. Let me re-examine this file.
Actually, looking at this file again, I don't see a typo on line 14. Let me move on to check if there are other issues.
26-30: Consider stronger validation for provider configuration.The current schema uses
.passthrough()which allows any additional properties. While this provides flexibility, it may allow invalid configurations to pass validation. Consider defining specific provider types or at least validating thetypefield against known values.z .object({ - type: z.string(), + type: z.enum(['path', 'other-known-types']), // or z.union for multiple types }) - .passthrough(), + .strict(), // or keep passthrough if truly neededpackages/core-generators/src/renderers/templates/utils/package-path-specifier.ts (1)
14-14: Fix typo in JSDoc comment."inclues" should be "includes".
- * @param packagePathSpecifier - The package path specifier is a path that inclues + * @param packagePathSpecifier - The package path specifier is a path that includespackages/sync/src/templates/extractor-v2/utils/group-template-files-by-type.ts (2)
29-34: Consider using a more robust key generation approach.Using
JSON.stringifyfor creating uniqueness keys is fragile and could break if object property order changes or if nested objects are involved. Consider using a more explicit key format.- const uniqueTemplateFiles = uniqBy(files, (f) => - JSON.stringify({ - g: f.metadata.generator, - t: f.metadata.template, - }), - ); + const uniqueTemplateFiles = uniqBy(files, (f) => + `${f.metadata.generator}::${f.metadata.template}` + );
64-86: Optimize duplicate detection algorithm.The current implementation has O(n) space complexity and could be simplified. The logic for tracking duplicates is correct but can be streamlined.
function findDuplicateNamesInGenerator( files: TemplateMetadataFileEntry[], ): TemplateMetadataFileEntry[] { - const duplicates: TemplateMetadataFileEntry[] = []; - const seen = new Map<string, TemplateMetadataFileEntry>(); + const seenKeys = new Map<string, TemplateMetadataFileEntry[]>(); for (const file of files) { const key = `${file.metadata.generator}:${file.metadata.name}`; - const existing = seen.get(key); - - if (existing) { - // Add both files as duplicates (if not already added) - if (!duplicates.includes(existing)) { - duplicates.push(existing); - } - duplicates.push(file); - } else { - seen.set(key, file); + if (!seenKeys.has(key)) { + seenKeys.set(key, []); } + seenKeys.get(key)!.push(file); } - return duplicates; + return Array.from(seenKeys.values()) + .filter(files => files.length > 1) + .flat(); }packages/core-generators/src/renderers/raw/render-raw-template-action.unit.test.ts (1)
79-79: Consider adding generator info to the second test.For consistency and to ensure the ID generation works correctly in both scenarios, consider providing
generatorInfoin the second test as well.- const output = await testAction(action); + const output = await testAction(action, { + generatorInfo: { + name: 'test-generator', + baseDirectory: '/root/pkg/test-generator', + }, + });packages/core-generators/src/renderers/text/utils.ts (2)
8-31: Consider making delimiter configuration more extensible.The current hardcoded approach works but could be made more maintainable with a configuration object approach.
+const DELIMITER_CONFIG = { + '.css': { start: '/* ', end: ' */' }, + '.gql': { start: '', end: '' }, +} as const; + +const DEFAULT_DELIMITERS = { start: '{{', end: '}}' } as const; + export function getTextTemplateDelimiters(filename: string): { start: string; end: string; } { - if (filename.endsWith('.css')) { - return { - start: '/* ', - end: ' */', - }; - } - - // no delimiters for gql files - if (filename.endsWith('.gql')) { - return { - start: '', - end: '', - }; - } - - return { - start: '{{', - end: '}}', - }; + const extension = Object.keys(DELIMITER_CONFIG).find(ext => + filename.endsWith(ext) + ); + + return extension ? DELIMITER_CONFIG[extension] : DEFAULT_DELIMITERS; }
34-42: Consider documenting regex behavior and adding input validation.The function documentation mentions checking for "non-alphanumeric characters" but the regex uses
\wwhich includes underscore. Consider clarifying the documentation and adding input validation./** - * Get the regex for a text template variable. We check for non-alphanumeric characters around the variable name. + * Get the regex for a text template variable. Uses word boundaries to ensure exact matches. + * Note: \w includes letters, digits, and underscore. * - * @param variable The variable to get the regex for. * @param value The value of the variable. * @returns The regex for the text template variable. */ export function getTextTemplateVariableRegExp(value: string): RegExp { + if (!value) { + throw new Error('Variable value cannot be empty'); + } return new RegExp(`(?<!\\w)${escapeRegExp(value)}(?!\\w)`, 'g'); }packages/core-generators/src/renderers/raw/render-raw-template-action.ts (2)
41-43: Add runtime validation for singleton templates with unexpected id.While the type system prevents this scenario, consider adding runtime validation for defensive programming:
if (template.fileOptions.kind === 'instance' && !id) { throw new Error('Instance template must have an id'); } +if (template.fileOptions.kind === 'singleton' && id) { + throw new Error('Singleton template should not have an id'); +}
59-59: Document or remove the deprecated template field.The comment indicates this field is deprecated, but it's unclear why it's still being set to an empty string. Consider documenting the deprecation timeline or removing it if no longer needed.
packages/sync/src/templates/extractor-v2/runner/template-extractor-file-container.unit.test.ts (2)
143-152: Consider using vi.importMock instead of vi.doMock for consistency.The dynamic mocking approach works but could be simplified using vi.importMock for better readability:
-// Mock fs.writeFile to track calls -const writeFileSpy = vi.fn(); -vi.doMock('node:fs/promises', async () => ({ - ...(await vi.importActual('node:fs/promises')), - writeFile: writeFileSpy, -})); +const writeFileSpy = vi.fn(); +const mockFs = await vi.importMock('node:fs/promises'); +mockFs.writeFile = writeFileSpy;
216-220: Verify Buffer handling in memfs serialization.The test assumes that memfs returns string content for Buffer inputs. Consider adding explicit Buffer comparison to ensure the test correctly validates binary data handling:
const files = vol.toJSON(); const fileContent = files[filePath]; -if (typeof fileContent === 'string') { - expect(Buffer.from(fileContent)).toEqual(contents); -} else { - throw new TypeError('Expected file content to be a string'); -} +// memfs serializes Buffers as strings, verify the round-trip +expect(Buffer.from(fileContent as string, 'utf8')).toEqual(contents);packages/sync/src/templates/metadata/read-template-metadata-files.ts (1)
42-71: Consider performance optimization for large projects.The current implementation processes all metadata files and their entries concurrently, which could be memory-intensive for large projects. Consider if batching or streaming would be beneficial.
For projects with many template files, consider adding a batch processing option:
+export async function readTemplateMetadataFiles( + outputDirectory: string, + options: { batchSize?: number } = {}, +): Promise<TemplateMetadataFileEntry[]> { + const { batchSize = 100 } = options;However, this optimization may not be necessary unless performance issues are observed in practice.
packages/utils/src/paths/safe-path-join.ts (2)
1-1: Consider adding path import fromnode:path/posixfor cross-platform consistency.To ensure consistent behavior across different platforms, especially when dealing with path operations that might be used in cross-platform scenarios, consider importing from
node:path/posixor handling platform-specific separators more explicitly.
72-78: Simplify the path containment check logic.The current implementation has redundant checks. After ensuring
parentWithSepends with a separator, the special case for exact match (lines 68-70) becomes unnecessary sinceresolved.startsWith(parentWithSep)would return false for exact matches.Simplify the logic:
- // Special case: if resolved is exactly the parent directory - if (resolved === normalizedParent) { - return resolved; - } - // Check if resolved path starts with parent directory - if (!resolved.startsWith(parentWithSep)) { + if (resolved !== normalizedParent && !resolved.startsWith(parentWithSep)) { throw new Error( `Path traversal detected: Resolved path "${resolved}" is outside parent directory "${normalizedParent}"`, ); }packages/sync/src/templates/extractor-v2/utils/update-extractor-template-entries.unit.test.ts (1)
122-125: Add type assertion for better type safety.The type assertion
as stringon line 123 could be avoided by checking the type first.Consider a safer approach:
const writtenContent = files.get(generatorJsonPath); expect(writtenContent).toBeDefined(); - const parsedContent = JSON.parse(writtenContent as string) as { + if (typeof writtenContent !== 'string') { + throw new Error('Expected written content to be a string'); + } + const parsedContent = JSON.parse(writtenContent) as { templates: Record<string, TemplateConfig>; };packages/sync/src/templates/extractor-v2/runner/initialize-template-extractor-plugins.ts (2)
144-150: Consider providing more context in the error message.When a plugin is not found in the map during sorting, it would be helpful to know which plugins were expected vs. which were found.
Enhance the error message:
return sortedNames.map((name: string) => { const plugin = pluginMap.get(name); if (!plugin) { - throw new Error(`Plugin ${name} not found in plugin map`); + throw new Error(`Plugin "${name}" not found in plugin map. Available plugins: ${[...pluginMap.keys()].join(', ')}`); } return plugin; });
161-167: Consider adding validation for hook names.The
registerHookmethod accepts a string literal type but doesn't validate at runtime. If this API is exposed to external plugins, runtime validation would be beneficial.Add runtime validation:
registerHook( hook: 'afterExtract' | 'afterWrite', callback: () => void | Promise<void>, ): void { + if (hook !== 'afterExtract' && hook !== 'afterWrite') { + throw new Error(`Invalid hook name: ${hook}. Valid hooks are: afterExtract, afterWrite`); + } allHooks[hook].push(callback); },packages/sync/src/templates/extractor-v2/runner/template-extractor-context.ts (1)
17-35: Use parameter properties &readonlyto reduce duplicationAll ctor parameters are later re-declared as mutable public fields, increasing surface area for accidental mutation.
-export class TemplateExtractorContext<…> { - constructor({ - configLookup, - logger, - outputDirectory, - plugins, - fileContainer, - }: { … }) { - this.configLookup = configLookup; - this.logger = logger; - this.outputDirectory = outputDirectory; - this.plugins = plugins; - this.fileContainer = fileContainer; - } +export class TemplateExtractorContext<…> { + constructor( + public readonly configLookup: TemplateExtractorConfigLookup, + public readonly logger: Logger, + public readonly outputDirectory: string, + public readonly plugins: Map<string, unknown>, + public readonly fileContainer: TemplateExtractorFileContainer, + ) {}This trims 20+ LOC and hardens the state against mutation.
packages/core-generators/src/renderers/text/render-text-template-file-action.ts (1)
76-86: No validation for extraneous variablesKeys in
variablesObjare only checked for theTPL_prefix. Unexpected variables silently pass through and are ignored if not present in the template, which may indicate a caller bug.Consider:
const unknownKeys = Object.keys(variablesObj).filter( (k) => !(k in templateVariables), ); if (unknownKeys.length) { throw new Error(`Unknown template variables: ${unknownKeys.join(', ')}`); }packages/sync/src/templates/extractor-v2/runner/template-extractor-file-container.ts (1)
44-52: Optimize change detection with early return for Buffer contents.Consider adding an early return for Buffer contents that are already in the correct format to avoid unnecessary Buffer conversion.
const contentsBuffer = Buffer.isBuffer(formattedContents) ? formattedContents : Buffer.from(formattedContents); const existingContents = await fs .readFile(filePath) .catch(handleFileNotFoundError); + if (!existingContents) { + // File doesn't exist, proceed with write + } else if (existingContents?.equals(contentsBuffer)) { return; }packages/core-generators/src/renderers/raw/types.ts (1)
54-58:createRawTemplateFileis an unnecessary no-op wrapper
The helper just returns its argument without any transformation or validation, so every call site could directly use theRawTemplateFileliteral instead. Unless you plan to extend this function later (e.g. add defaults or runtime checks), consider inlining it to keep the API surface minimal.-export function createRawTemplateFile( - templateFile: RawTemplateFile, -): RawTemplateFile { - return templateFile; -} +// If no additional processing is required, remove this wrapper and +// construct `RawTemplateFile` objects inline at call sites.packages/core-generators/src/renderers/templates/plugins/template-paths/paths-file.ts (1)
10-21: Import order violates configured ESLint rule
ESLint (perfectionist/sort-imports) expects#src/renderers/templates/utils/index.jsto appear before#src/renderers/typescript/index.js. Re-ordering maintains rule conformity and avoids CI noise.-import { - renderTsCodeFileTemplate, - TsCodeUtils, - tsTemplate, -} from '#src/renderers/typescript/index.js'; -import { +import { getGeneratedTemplateConstantName, getGeneratedTemplateExportName, getGeneratedTemplateInterfaceName, getGeneratedTemplateProviderName, resolvePackagePathSpecifier, } from '#src/renderers/templates/utils/index.js'; +import { + renderTsCodeFileTemplate, + TsCodeUtils, + tsTemplate, +} from '#src/renderers/typescript/index.js';🧰 Tools
🪛 ESLint
[error] 15-21: Expected "#src/renderers/templates/utils/index.js" to come before "#src/renderers/typescript/index.js".
(perfectionist/sort-imports)
packages/sync/src/templates/extractor-v2/runner/template-extractor-api.ts (1)
12-15:extractorNameis stored but never usedThe constructor captures
extractorName, yet no other method references it.
Either wire this through where required (e.g. for error messages / metrics) or drop the parameter to avoid dead code.packages/core-generators/src/renderers/templates/plugins/typed-templates-file.ts (2)
37-46:addTemplatemutates and immediately re-sets the same array
generatorTemplates.get()already returns a mutable array reference, so the subsequentgeneratorTemplates.set()is redundant and slightly misleading.-const templates = generatorTemplates.get(generatorName) ?? []; -templates.push(template); -generatorTemplates.set(generatorName, templates); +const list = generatorTemplates.get(generatorName); +if (list) { + list.push(template); +} else { + generatorTemplates.set(generatorName, [template]); +}This avoids unnecessary
Mapchurn and makes intent clearer.
48-78: Potential duplicate exports and unpredictable orderingIf two extractors register the same
exportName, the merged fragments will silently collide, and the later one wins.
At minimum, de-duplicate and warn:if (templatesFragment.has(exportName)) { console.warn(`[typed-templates-file] Duplicate export "${exportName}" for generator "${generatorName}".`); }Also, sorting
templateExportsbefore joining provides deterministic output and diff-friendly files.packages/project-builder-server/src/template-extractor/run-template-extractor-v2.ts (1)
16-20: Consider making generator packages configurable.The
GENERATOR_PACKAGESarray is hardcoded, which limits extensibility. Consider making this configurable through environment variables or configuration files to support custom generator packages.-const GENERATOR_PACKAGES = [ - '@baseplate-dev/core-generators', - '@baseplate-dev/fastify-generators', - '@baseplate-dev/react-generators', -]; +const DEFAULT_GENERATOR_PACKAGES = [ + '@baseplate-dev/core-generators', + '@baseplate-dev/fastify-generators', + '@baseplate-dev/react-generators', +]; + +const GENERATOR_PACKAGES = process.env.BASEPLATE_GENERATOR_PACKAGES + ? process.env.BASEPLATE_GENERATOR_PACKAGES.split(',').map(pkg => pkg.trim()) + : DEFAULT_GENERATOR_PACKAGES;packages/sync/src/templates/extractor-v2/run-template-file-extractors.ts (1)
25-32: JSDoc parameter name mismatches implementationDocs say
outputDirectories(plural) while the function accepts a singleoutputDirectory: string.- * @param outputDirectories - The output directories to run the extractors on + * @param outputDirectory - The output directory to run the extractors onKeeping API docs accurate avoids confusion for integrators and IDE hints.
packages/sync/src/templates/extractor-v2/runner/template-extractor-plugin.ts (2)
25-27: Document hook execution order in the interface.The comment mentions that
afterWriteis "called in reverse order of dependencies" but this critical behavior is only documented in a comment. Consider adding this to theTemplateExtractorPluginApiinterface documentation or enforcing it through the type system.
30-35: Consider expanding the plugin API for future extensibility.The
TemplateExtractorPluginApiinterface currently only exposes a singleregisterHookmethod. This might limit future extensibility if plugins need access to other functionality like logging, configuration, or inter-plugin communication.Consider adding methods like:
getLogger(): Loggerfor consistent logginggetConfig(): unknownfor plugin-specific configurationemit(event: string, data: unknown): voidfor custom eventspackages/core-generators/src/renderers/text/types.ts (2)
90-94: Clarify the default generic constraint.The default generic constraint
Record<never, TextTemplateFileVariable>creates an empty record, which might be confusing. Consider using a more explicit type or adding a comment explaining this pattern.export interface TextTemplateFile< - T extends Record<string, TextTemplateFileVariable> = Record< - never, - TextTemplateFileVariable - >, + T extends Record<string, TextTemplateFileVariable> = Record<string, never>, > extends TemplateFileBase {
56-62: Remove redundant inline comment.The comment on line 59 is redundant as the property name
valueis self-explanatory.textTemplateFileVariableSchema.extend({ - // The value of the variable. value: z.string(), }),packages/sync/src/templates/extractor-v2/runner/template-file-extractor.ts (2)
100-109: Consider standardizing on async methods.The return type allows both synchronous and asynchronous implementations, which could lead to inconsistent usage patterns. Consider standardizing on
Promisereturns for consistency.extractTemplateMetadataEntries( files: TemplateFileExtractorSourceFile<z.infer<TOutputTemplateMetadata>>[], context: TemplateExtractorContext<TPluginDependencies>, - ): - | Promise< - TemplateFileExtractorMetadataEntry< - z.infer<TGeneratorTemplateMetadata> - >[] - > - | TemplateFileExtractorMetadataEntry<z.infer<TGeneratorTemplateMetadata>>[]; + ): Promise< + TemplateFileExtractorMetadataEntry< + z.infer<TGeneratorTemplateMetadata> + >[] + >;
135-139: Standardize return type for consistency.Similar to
extractTemplateMetadataEntries, this method allows both sync and async returns. For API consistency, consider standardizing onPromise<void>.writeGeneratedFiles( generatorNames: string[], context: TemplateExtractorContext<TPluginDependencies>, api: TemplateExtractorApi, - ): Promise<void> | void; + ): Promise<void>;packages/sync/src/templates/extractor-v2/configs/template-extractor-config-lookup.ts (4)
65-65: Inconsistent cache key format between extractors and providersThe cache keys use different separators:
#for extractors (${packageName}#${config.name}) and:for providers (${packageName}:${providerName}). This inconsistency could lead to confusion and maintenance issues.Consider standardizing the cache key format. Apply this diff to use consistent separators:
- this.extractorConfigCache.set(`${packageName}#${config.name}`, { + this.extractorConfigCache.set(`${packageName}:${config.name}`, {Also applies to: 90-90
190-203: Remove unnecessary type assertionThe type assertion
as z.infer<T>on line 201 is redundant sinceparse()already returns the correctly typed value.- config: generatorTemplateMetadataSchema.parse(template) as z.infer<T>, + config: generatorTemplateMetadataSchema.parse(template),
114-126: Consider adding ignore patterns for common build directoriesWhile
gitignore: truehelps filter out version-controlled ignore patterns, you might want to explicitly exclude common build directories to avoid picking up generated or temporary config files.const configFiles = await globby( [ path.join(packagePath, '**/extractor.json'), path.join(packagePath, '**/providers.json'), ], { cwd: packagePath, absolute: true, onlyFiles: true, fs: fsAdapter, gitignore: true, + ignore: ['**/node_modules/**', '**/dist/**', '**/build/**', '**/.next/**'], }, );
35-262: Consider adding refresh capability for dynamic updatesThe class currently doesn't provide a way to re-index packages or refresh configurations after initialization. This could be limiting in scenarios where configurations might change during runtime (e.g., in development with hot reloading).
Consider adding methods to:
- Re-index a specific package
- Clear and re-initialize all caches
- Watch for configuration file changes
This would make the system more flexible for development environments where configurations might change without restarting the process.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
packages/fastify-generators/src/generators/core/fastify-scripts/generated/raw-templates.tsis excluded by!**/generated/**packages/fastify-generators/src/generators/core/fastify-scripts/generated/template-paths.tsis excluded by!**/generated/**packages/fastify-generators/src/generators/core/fastify-scripts/generated/typed-templates.tsis excluded by!**/generated/**packages/fastify-generators/src/generators/core/readme/generated/template-paths.tsis excluded by!**/generated/**packages/fastify-generators/src/generators/core/readme/generated/text-templates.tsis excluded by!**/generated/**packages/fastify-generators/src/generators/core/readme/generated/typed-templates.tsis excluded by!**/generated/**
📒 Files selected for processing (96)
.changeset/template-extractor-v2-foundation.md(1 hunks).cursor/rules/tests.mdc(1 hunks)CLAUDE.md(2 hunks)packages/core-generators/src/generators/docker/docker-compose/docker-compose.generator.ts(2 hunks)packages/core-generators/src/generators/index.ts(1 hunks)packages/core-generators/src/generators/metadata/index.ts(1 hunks)packages/core-generators/src/generators/metadata/path-roots/path-roots.generator.ts(1 hunks)packages/core-generators/src/generators/node/_composers/node.ts(2 hunks)packages/core-generators/src/generators/node/node/node.generator.ts(2 hunks)packages/core-generators/src/generators/node/node/node.generator.unit.test.ts(1 hunks)packages/core-generators/src/providers/project.ts(1 hunks)packages/core-generators/src/providers/providers.json(1 hunks)packages/core-generators/src/renderers/index.ts(1 hunks)packages/core-generators/src/renderers/raw/index.ts(1 hunks)packages/core-generators/src/renderers/raw/raw-template-file-extractor.ts(1 hunks)packages/core-generators/src/renderers/raw/render-raw-template-action.ts(1 hunks)packages/core-generators/src/renderers/raw/render-raw-template-action.unit.test.ts(1 hunks)packages/core-generators/src/renderers/raw/types.ts(1 hunks)packages/core-generators/src/renderers/schemas/template-file-options.ts(1 hunks)packages/core-generators/src/renderers/templates/index.ts(1 hunks)packages/core-generators/src/renderers/templates/plugins/barrel-import.ts(1 hunks)packages/core-generators/src/renderers/templates/plugins/barrel-import.unit.test.ts(1 hunks)packages/core-generators/src/renderers/templates/plugins/index.ts(1 hunks)packages/core-generators/src/renderers/templates/plugins/template-paths/index.ts(1 hunks)packages/core-generators/src/renderers/templates/plugins/template-paths/paths-file.ts(1 hunks)packages/core-generators/src/renderers/templates/plugins/template-paths/template-paths.plugin.ts(1 hunks)packages/core-generators/src/renderers/templates/plugins/typed-templates-file.ts(1 hunks)packages/core-generators/src/renderers/templates/utils/generated-template-file-names.ts(1 hunks)packages/core-generators/src/renderers/templates/utils/index.ts(1 hunks)packages/core-generators/src/renderers/templates/utils/package-path-specifier.ts(1 hunks)packages/core-generators/src/renderers/text/index.ts(1 hunks)packages/core-generators/src/renderers/text/render-text-template-file-action.ts(1 hunks)packages/core-generators/src/renderers/text/render-text-template-file-action.unit.test.ts(1 hunks)packages/core-generators/src/renderers/text/render-text-template-group-action.ts(1 hunks)packages/core-generators/src/renderers/text/text-template-file-extractor.ts(1 hunks)packages/core-generators/src/renderers/text/types.ts(1 hunks)packages/core-generators/src/renderers/text/utils.ts(1 hunks)packages/core-generators/src/renderers/text/utils.unit.test.ts(1 hunks)packages/core-generators/src/renderers/typescript/actions/render-ts-template-file-action.ts(1 hunks)packages/fastify-generators/src/generators/core/app-module/app-module.generator.ts(4 hunks)packages/fastify-generators/src/generators/core/fastify-scripts/extractor.json(1 hunks)packages/fastify-generators/src/generators/core/fastify-scripts/fastify-scripts.generator.ts(3 hunks)packages/fastify-generators/src/generators/core/readme/extractor.json(1 hunks)packages/fastify-generators/src/generators/core/readme/readme.generator.ts(2 hunks)packages/fastify-generators/src/generators/prisma/prisma/prisma.generator.ts(2 hunks)packages/fastify-generators/src/generators/vitest/prisma-vitest/prisma-vitest.generator.ts(3 hunks)packages/project-builder-cli/src/commands/extract-templates.ts(2 hunks)packages/project-builder-server/src/sync/build-project.ts(1 hunks)packages/project-builder-server/src/template-extractor/index.ts(1 hunks)packages/project-builder-server/src/template-extractor/run-template-extractor-v2.ts(1 hunks)packages/project-builder-server/src/template-extractor/run-template-extractor.ts(1 hunks)packages/project-builder-test/src/runner/discover-tests.ts(1 hunks)packages/react-generators/src/generators/core/react-routes/react-routes.generator.ts(2 hunks)packages/react-generators/src/generators/core/react/react.generator.ts(3 hunks)packages/sync/package.json(1 hunks)packages/sync/src/output/generator-task-output.ts(1 hunks)packages/sync/src/templates/extractor-v2/configs/extractor-config.schema.ts(1 hunks)packages/sync/src/templates/extractor-v2/configs/index.ts(1 hunks)packages/sync/src/templates/extractor-v2/configs/providers-config.schema.ts(1 hunks)packages/sync/src/templates/extractor-v2/configs/template-extractor-config-lookup.ts(1 hunks)packages/sync/src/templates/extractor-v2/configs/template-extractor-config-lookup.unit.test.ts(1 hunks)packages/sync/src/templates/extractor-v2/configs/try-create-extractor-json.ts(1 hunks)packages/sync/src/templates/extractor-v2/configs/try-create-extractor-json.unit.test.ts(1 hunks)packages/sync/src/templates/extractor-v2/index.ts(1 hunks)packages/sync/src/templates/extractor-v2/run-template-file-extractors.ts(1 hunks)packages/sync/src/templates/extractor-v2/runner/index.ts(1 hunks)packages/sync/src/templates/extractor-v2/runner/initialize-template-extractor-plugins.ts(1 hunks)packages/sync/src/templates/extractor-v2/runner/initialize-template-extractor-plugins.unit.test.ts(1 hunks)packages/sync/src/templates/extractor-v2/runner/template-extractor-api.ts(1 hunks)packages/sync/src/templates/extractor-v2/runner/template-extractor-context.ts(1 hunks)packages/sync/src/templates/extractor-v2/runner/template-extractor-file-container.ts(1 hunks)packages/sync/src/templates/extractor-v2/runner/template-extractor-file-container.unit.test.ts(1 hunks)packages/sync/src/templates/extractor-v2/runner/template-extractor-plugin.ts(1 hunks)packages/sync/src/templates/extractor-v2/runner/template-file-extractor.ts(1 hunks)packages/sync/src/templates/extractor-v2/utils/group-template-files-by-type.ts(1 hunks)packages/sync/src/templates/extractor-v2/utils/group-template-files-by-type.unit.test.ts(1 hunks)packages/sync/src/templates/extractor-v2/utils/index.ts(1 hunks)packages/sync/src/templates/extractor-v2/utils/update-extractor-template-entries.ts(1 hunks)packages/sync/src/templates/extractor-v2/utils/update-extractor-template-entries.unit.test.ts(1 hunks)packages/sync/src/templates/extractor/run-template-file-extractors.ts(1 hunks)packages/sync/src/templates/metadata/index.ts(1 hunks)packages/sync/src/templates/metadata/metadata.ts(1 hunks)packages/sync/src/templates/metadata/read-template-metadata-files.ts(1 hunks)packages/sync/src/templates/metadata/read-template-metadata-files.unit.test.ts(1 hunks)packages/sync/src/templates/raw-template/render-raw-template-action.ts(1 hunks)packages/sync/src/templates/text-template/render-text-template-file-action.ts(1 hunks)packages/sync/src/templates/utils/formatter.ts(2 hunks)packages/sync/src/templates/utils/read-template-file-source.ts(1 hunks)packages/sync/src/utils/canonical-path.ts(2 hunks)packages/utils/src/paths/index.ts(1 hunks)packages/utils/src/paths/normalize-path-separators.ts(1 hunks)packages/utils/src/paths/normalize-path-separators.unit.test.ts(1 hunks)packages/utils/src/paths/safe-path-join.ts(1 hunks)packages/utils/src/paths/safe-path-join.unit.test.ts(1 hunks)packages/utils/src/validators/case-validators.ts(1 hunks)plugins/plugin-storage/src/generators/fastify/storage-module/extractor.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
.cursor/rules/tests.mdc (2)
Learnt from: kingston
PR: halfdomelabs/baseplate#510
File: packages/project-builder-server/src/sync/conflict-file-monitor.test.ts:19-24
Timestamp: 2025-04-23T06:44:30.952Z
Learning: When testing file operations in this codebase, the pattern is to use Vitest automocks for 'node:fs' and 'node:fs/promises' (without explicit implementation replacement) while populating a virtual filesystem with vol.fromJSON() from memfs. File operations in tests are performed directly via vol.promises methods.
Learnt from: kingston
PR: halfdomelabs/baseplate#510
File: packages/project-builder-server/src/sync/conflict-file-monitor.test.ts:19-24
Timestamp: 2025-04-23T06:44:30.952Z
Learning: In this codebase, when testing file operations, Vitest's automocks for 'node:fs' and 'node:fs/promises' are used in conjunction with memfs, but without replacing the mock implementation explicitly in each test file. The virtual filesystem is populated using vol.fromJSON or similar methods, which works seamlessly with the automocks.
packages/sync/src/templates/extractor-v2/runner/template-extractor-file-container.unit.test.ts (2)
Learnt from: kingston
PR: halfdomelabs/baseplate#510
File: packages/project-builder-server/src/sync/conflict-file-monitor.test.ts:19-24
Timestamp: 2025-04-23T06:44:30.952Z
Learning: When testing file operations in this codebase, the pattern is to use Vitest automocks for 'node:fs' and 'node:fs/promises' (without explicit implementation replacement) while populating a virtual filesystem with vol.fromJSON() from memfs. File operations in tests are performed directly via vol.promises methods.
Learnt from: kingston
PR: halfdomelabs/baseplate#510
File: packages/project-builder-server/src/sync/conflict-file-monitor.test.ts:19-24
Timestamp: 2025-04-23T06:44:30.952Z
Learning: In this codebase, when testing file operations, Vitest's automocks for 'node:fs' and 'node:fs/promises' are used in conjunction with memfs, but without replacing the mock implementation explicitly in each test file. The virtual filesystem is populated using vol.fromJSON or similar methods, which works seamlessly with the automocks.
🧬 Code Graph Analysis (24)
packages/fastify-generators/src/generators/prisma/prisma/prisma.generator.ts (1)
packages/core-generators/src/providers/project.ts (1)
packageInfoProvider(18-19)
packages/core-generators/src/generators/docker/docker-compose/docker-compose.generator.ts (1)
packages/core-generators/src/providers/project.ts (1)
packageInfoProvider(18-19)
packages/utils/src/paths/normalize-path-separators.unit.test.ts (1)
packages/utils/src/paths/normalize-path-separators.ts (1)
normalizePathSeparators(8-10)
packages/core-generators/src/generators/node/_composers/node.ts (1)
packages/core-generators/src/generators/metadata/path-roots/path-roots.generator.ts (1)
pathRootsGenerator(33-77)
packages/react-generators/src/generators/core/react-routes/react-routes.generator.ts (2)
packages/core-generators/src/generators/metadata/path-roots/path-roots.generator.ts (1)
pathRootsProvider(26-27)packages/react-generators/src/providers/routes.ts (4)
reactRoutesProvider(26-27)reactRoutesReadOnlyProvider(34-37)ReactRoute(11-17)ReactRouteLayout(5-9)
packages/core-generators/src/renderers/templates/plugins/template-paths/template-paths.plugin.ts (6)
packages/sync/src/templates/extractor-v2/runner/template-extractor-plugin.ts (1)
createTemplateExtractorPlugin(55-63)packages/core-generators/src/renderers/templates/plugins/barrel-import.ts (1)
templateExtractorBarrelImportPlugin(122-178)packages/core-generators/src/renderers/templates/plugins/template-paths/paths-file.ts (1)
writePathMapFile(199-243)packages/utils/src/fs/read-json-with-schema.ts (1)
readJsonWithSchema(13-36)packages/sync/src/utils/canonical-path.ts (1)
toCanonicalPath(28-39)packages/utils/src/paths/posix-join.ts (1)
posixJoin(10-12)
packages/sync/src/templates/utils/read-template-file-source.ts (1)
packages/sync/src/templates/extractor/template-file-extractor-test-utils.ts (1)
templatePath(66-77)
packages/core-generators/src/renderers/text/utils.unit.test.ts (1)
packages/core-generators/src/renderers/text/utils.ts (1)
getTextTemplateVariableRegExp(40-42)
packages/fastify-generators/src/generators/core/app-module/app-module.generator.ts (1)
packages/core-generators/src/generators/metadata/path-roots/path-roots.generator.ts (1)
pathRootsProvider(26-27)
packages/react-generators/src/generators/core/react/react.generator.ts (1)
packages/core-generators/src/providers/project.ts (1)
packageInfoProvider(18-19)
packages/sync/src/utils/canonical-path.ts (2)
packages/utils/src/paths/normalize-path-separators.ts (1)
normalizePathSeparators(8-10)packages/utils/src/paths/posix-join.ts (1)
posixJoin(10-12)
packages/project-builder-cli/src/commands/extract-templates.ts (2)
packages/project-builder-cli/src/services/schema-parser-context.ts (1)
createSchemaParserContext(13-18)packages/project-builder-server/src/template-extractor/run-template-extractor-v2.ts (1)
runTemplateExtractorsForProjectV2(60-103)
packages/fastify-generators/src/generators/vitest/prisma-vitest/prisma-vitest.generator.ts (1)
packages/core-generators/src/providers/project.ts (1)
packageInfoProvider(18-19)
packages/core-generators/src/providers/project.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(219-224)
packages/sync/src/templates/extractor-v2/utils/update-extractor-template-entries.ts (4)
packages/sync/src/templates/extractor-v2/runner/template-file-extractor.ts (1)
TemplateFileExtractorMetadataEntry(35-54)packages/sync/src/templates/extractor-v2/runner/template-extractor-context.ts (1)
TemplateExtractorContext(13-73)packages/utils/src/objects/sort-object-keys.ts (1)
sortObjectKeys(7-11)packages/utils/src/json/stringify-pretty-compact.ts (1)
stringifyPrettyCompact(103-194)
packages/sync/src/templates/extractor-v2/runner/template-extractor-context.ts (2)
packages/sync/src/templates/extractor-v2/runner/template-extractor-plugin.ts (2)
TemplateExtractorPluginDependencies(6-6)InferTemplateExtractorPluginDependency(11-20)packages/sync/src/templates/extractor-v2/runner/template-extractor-file-container.ts (1)
TemplateExtractorFileContainer(10-73)
packages/core-generators/src/generators/node/node/node.generator.ts (3)
packages/sync/src/generators/generators.ts (1)
createGeneratorTask(196-204)packages/core-generators/src/providers/project.ts (1)
packageInfoProvider(18-19)packages/core-generators/src/generators/metadata/path-roots/path-roots.generator.ts (1)
pathRootsProvider(26-27)
packages/sync/src/templates/extractor-v2/configs/extractor-config.schema.ts (1)
packages/utils/src/validators/case-validators.ts (1)
CASE_VALIDATORS(23-48)
packages/fastify-generators/src/generators/core/readme/readme.generator.ts (4)
packages/fastify-generators/src/generators/core/readme/generated/template-paths.ts (1)
CORE_README_PATHS(25-28)packages/sync/src/generators/generators.ts (1)
createGeneratorTask(196-204)packages/core-generators/src/providers/project.ts (1)
packageInfoProvider(18-19)packages/fastify-generators/src/generators/core/readme/generated/typed-templates.ts (1)
CORE_README_TEMPLATES(13-13)
packages/sync/src/templates/extractor-v2/runner/template-extractor-file-container.ts (1)
packages/sync/src/templates/utils/formatter.ts (1)
formatGeneratedTemplateContents(48-59)
packages/sync/src/templates/extractor-v2/runner/template-extractor-plugin.ts (1)
packages/sync/src/templates/extractor-v2/runner/template-extractor-context.ts (1)
TemplateExtractorContext(13-73)
packages/sync/src/templates/metadata/metadata.ts (1)
packages/utils/src/validators/case-validators.ts (1)
CASE_VALIDATORS(23-48)
packages/core-generators/src/renderers/raw/types.ts (4)
packages/sync/src/templates/extractor-v2/configs/extractor-config.schema.ts (1)
templateConfigSchema(7-18)packages/core-generators/src/renderers/schemas/template-file-options.ts (2)
templateFileOptionsSchema(3-20)TemplateFileOptions(22-22)packages/sync/src/templates/metadata/metadata.ts (1)
templateFileMetadataBaseSchema(4-22)packages/sync/src/templates/types.ts (1)
TemplateFileBase(21-34)
packages/sync/src/templates/extractor-v2/runner/template-file-extractor.ts (5)
packages/sync/src/templates/metadata/metadata.ts (2)
TemplateFileMetadataBase(24-26)templateFileMetadataBaseSchema(4-22)packages/sync/src/templates/extractor-v2/configs/extractor-config.schema.ts (2)
TemplateConfig(46-46)templateConfigSchema(7-18)packages/sync/src/templates/extractor-v2/runner/template-extractor-plugin.ts (1)
TemplateExtractorPluginDependencies(6-6)packages/sync/src/templates/extractor-v2/runner/template-extractor-context.ts (1)
TemplateExtractorContext(13-73)packages/sync/src/templates/extractor-v2/runner/template-extractor-api.ts (1)
TemplateExtractorApi(11-102)
🪛 ESLint
packages/core-generators/src/renderers/templates/plugins/template-paths/paths-file.ts
[error] 15-21: Expected "#src/renderers/templates/utils/index.js" to come before "#src/renderers/typescript/index.js".
(perfectionist/sort-imports)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test E2E
- GitHub Check: test
- GitHub Check: Lint
🔇 Additional comments (136)
packages/project-builder-test/src/runner/discover-tests.ts (1)
18-18: Good improvement to respect .gitignore rules.Adding
gitignore: trueensures that test discovery respects.gitignorerules, preventing processing of files that should be excluded and improving performance.packages/sync/src/templates/extractor/run-template-file-extractors.ts (1)
54-55: Verify the pattern and cwd combination works as intended.The additions of
gitignore: trueandcwd: outputDirectoryare good improvements. However, please verify that using an absolute path pattern (path.join(outputDirectory, '**', TEMPLATE_METADATA_FILENAME)) combined withcwd: outputDirectoryproduces the expected results, as this combination might be redundant.Consider testing or using a relative pattern like
'**/' + TEMPLATE_METADATA_FILENAMEsincecwdis already set tooutputDirectory.plugins/plugin-storage/src/generators/fastify/storage-module/extractor.json (1)
1-3: Minimal extractor configuration is appropriate for the foundation phase.The JSON structure correctly follows the expected format for the new Template Extractor V2 system. The minimal configuration with just the generator name is suitable for this foundational implementation phase.
.changeset/template-extractor-v2-foundation.md (1)
1-5: Well-structured changeset documentation.The changeset correctly documents the foundational infrastructure addition with appropriate patch-level versioning for the
@baseplate-dev/syncpackage. The description accurately reflects the scope of changes introduced in this PR.packages/core-generators/src/renderers/templates/plugins/template-paths/index.ts (1)
1-1: LGTM! Clean barrel export implementation.The barrel export follows standard conventions and provides a clean entry point for the template-paths plugin.
packages/sync/src/templates/text-template/render-text-template-file-action.ts (1)
63-63:Details
✅ Verification successful
Property rename improves semantic clarity.
The rename from
hasManualIdtoisInstancebetter represents the concept of whether a file is an instance of a template vs a singleton. The boolean logic remains consistent.Let me verify this property rename is consistently applied across the codebase:
🏁 Script executed:
#!/bin/bash # Description: Verify the hasManualId to isInstance property rename is consistent across the codebase # Expected: No remaining occurrences of hasManualId in similar contexts echo "Searching for any remaining hasManualId usages:" rg "hasManualId" --type ts --type js -A 2 -B 2 echo -e "\nSearching for isInstance usages to verify consistency:" rg "isInstance.*:" --type ts --type js -A 2 -B 2Length of output: 3416
Consistent Rename Verified Across Codebase
A global search confirms there are no remaining
hasManualIdreferences andisInstancehas been applied uniformly in all relevant files (sync templates, core generators, and output types). The boolean logic (!!idor checkingtemplate.fileOptions.kind === 'instance') remains intact in each location.Approving this refactoring as complete.
packages/core-generators/src/renderers/typescript/actions/render-ts-template-file-action.ts (1)
133-133: Consistent property rename across template renderers.The
hasManualIdtoisInstancerename is consistently applied here, maintaining semantic alignment with the text template renderer changes.packages/core-generators/src/generators/index.ts (1)
2-2:Details
✅ Verification successful
New metadata generator export properly integrated.
The export follows the established pattern and maintains alphabetical ordering. This appropriately exposes the new metadata generator functionality for the Template Extractor V2 system.
Let me verify the exported metadata module exists and has proper exports:
🏁 Script executed:
#!/bin/bash # Description: Verify the metadata module exists and check its exports # Expected: The module should exist and export metadata-related functionality echo "Checking if the metadata index file exists:" fd "index\.(ts|js)" packages/core-generators/src/generators/metadata --exec ls -la {} echo -e "\nChecking metadata module exports:" fd "index\.(ts|js)" packages/core-generators/src/generators/metadata --exec cat {} echo -e "\nChecking for path-roots generator mentioned in the summary:" fd "path-roots" packages/core-generators/src/generators/metadata --type fLength of output: 808
Metadata generator export verified and approved
- packages/core-generators/src/generators/metadata/index.ts exists and contains:
export * from './path-roots/path-roots.generator.js';- packages/core-generators/src/generators/metadata/path-roots/path-roots.generator.ts is present, confirming the exported generator.
No further changes needed.
packages/core-generators/src/renderers/templates/index.ts (1)
1-2: LGTM! Clean barrel export implementation.The barrel export pattern is correctly implemented and aligns with the project's adoption of consolidated exports for better import organization.
packages/sync/src/templates/raw-template/render-raw-template-action.ts (1)
34-34:Details
✅ Verification successful
Verify property name consistency across the codebase.
The property rename from
hasManualIdtoisInstancemaintains the same logic while improving semantics. Ensure all references to this property have been updated consistently throughout the codebase.
🏁 Script executed:
#!/bin/bash # Verify that all references to hasManualId have been updated to isInstance echo "Searching for any remaining hasManualId references..." rg "hasManualId" --type ts echo "Confirming isInstance usage in TemplateMetadataOptions..." rg "isInstance.*TemplateMetadataOptions|TemplateMetadataOptions.*isInstance" --type ts -A 3 -B 3Length of output: 367
🏁 Script executed:
#!/bin/bash echo "Locating the definition of TemplateMetadataOptions..." rg "interface TemplateMetadataOptions" -n --type ts -A5 echo "Searching for all occurrences of 'isInstance' to verify consistent usage..." rg -n "isInstance" --type tsLength of output: 1666
Property rename is consistent across the codebase.
All occurrences of
hasManualIdhave been removed and replaced withisInstance. TheTemplateMetadataOptionsinterface in
packages/sync/src/output/generator-task-output.tsnow declaresisInstance: boolean, and every renderer and build task (in bothcore-generatorsandsyncpackages) referencesisInstanceinstead ofhasManualId. No broken or stale references remain. Approving this refactoring.packages/sync/src/templates/metadata/index.ts (1)
3-3: LGTM! Consistent with existing export pattern.The addition of the
read-template-metadata-files.jsexport maintains consistency with the existing barrel pattern and supports the Template Extractor V2 infrastructure.packages/utils/src/paths/index.ts (1)
2-2:Details
✅ Verification successful
Verify the exported modules exist and are properly tested.
The addition of
normalize-path-separators.jsandsafe-path-join.jsexports follows the established pattern. These utilities are crucial for cross-platform path handling and security in the Template Extractor V2 system.Also applies to: 4-4
🏁 Script executed:
#!/bin/bash # Verify the exported modules exist and have corresponding tests echo "Checking if the exported modules exist..." fd "normalize-path-separators\.(ts|js)" packages/utils/src/paths/ fd "safe-path-join\.(ts|js)" packages/utils/src/paths/ echo "Checking for corresponding test files..." fd "normalize-path-separators.*test\.(ts|js)" packages/utils/src/paths/ fd "safe-path-join.*test\.(ts|js)" packages/utils/src/paths/Length of output: 659
Path utility exports and tests confirmed
Both
packages/utils/src/paths/normalize-path-separators.tsandpackages/utils/src/paths/safe-path-join.tsare present, each with a corresponding unit test (*.unit.test.ts). No further action needed.packages/sync/src/templates/extractor-v2/utils/index.ts (1)
1-1:Details
✅ Verification successful
LGTM! Verify the referenced module exists.
The barrel export follows standard patterns and correctly uses the
.jsextension for TypeScript imports.Please verify that the referenced module exists and is properly integrated:
🏁 Script executed:
#!/bin/bash # Description: Verify the group-template-files-by-type module exists and is used # Check if the referenced module exists fd "group-template-files-by-type" --type f # Check usage of this barrel export rg -A 5 "from.*templates/extractor-v2/utils"Length of output: 257
LGTM – Referenced module confirmed
I’ve verified that
group-template-files-by-type.ts(and its unit test) exists underpackages/sync/src/templates/extractor-v2/utils. The.jsextension in the barrel export is intentional for TypeScript’s compiled output, so the export is correct.packages/core-generators/src/generators/metadata/index.ts (1)
1-1:Details
✅ Verification successful
LGTM! Clean integration of path roots generator.
The export properly integrates the
pathRootsGeneratorinto the metadata generators barrel, enabling access through the metadata package index.Verify the path roots generator integration across packages:
🏁 Script executed:
#!/bin/bash # Description: Verify path roots generator exists and is integrated properly # Check if the path-roots generator exists fd "path-roots.generator" --type f # Check usage of pathRootsGenerator/pathRootsProvider across packages rg -A 3 "pathRoots(Generator|Provider)"Length of output: 5103
✅ Path Roots Generator Fully Integrated Across Packages
Verified that the
path-roots.generatorexists and that bothpathRootsGeneratorandpathRootsProviderare correctly imported and used in:
packages/core-generators/src/generators/metadata/path-roots/path-roots.generator.tspackages/react-generators/src/generators/core/react-routes/react-routes.generator.tspackages/fastify-generators/src/generators/core/app-module/app-module.generator.tspackages/core-generators/src/generators/node/node/node.generator.tspackages/core-generators/src/generators/node/_composers/node.tsAll imports use the
.jsextension as expected in a TypeScript project targeting ES modules. No further changes are required.CLAUDE.md (2)
32-32: Good addition to code style guidelines.The barrel exports preference aligns well with the new index.ts files being introduced across the codebase and will help maintain consistency.
99-106: Valuable testing guidance for globby integration.This addition provides clear, actionable guidance for integrating globby with mocked filesystem operations, which will help prevent common testing issues.
packages/core-generators/src/generators/node/node/node.generator.unit.test.ts (1)
9-17: Test updates properly reflect the API refactoring.The changes correctly update the test to use the new
packageInfoprovider and its corresponding API methods while maintaining the same test coverage and logic.packages/fastify-generators/src/generators/prisma/prisma/prisma.generator.ts (3)
7-7: Import change aligns with the provider refactoring.The change from
projectProvidertopackageInfoProvideris consistent with the broader API refactoring and matches the new provider definitions.
104-106: Dependency and parameter updates are correct.The changes properly update the task to use the new
packageInfoProviderdependency and corresponding parameter, maintaining functionality while using the updated API.
109-109: Method call correctly uses the new API.The change from
project.getProjectName()topackageInfo.getPackageName()maintains the same functionality while using the more specific and accurate API method.packages/sync/src/templates/extractor-v2/configs/index.ts (1)
1-3: Well-structured barrel exports for extractor configuration.The barrel exports provide a clean API boundary for the extractor-v2 configuration system and follow the best practices outlined in the updated documentation.
packages/core-generators/src/renderers/templates/utils/index.ts (1)
1-3: LGTM! Clean barrel export implementation.The barrel export pattern is well-implemented and centralizes access to template utility functions, improving the module's organization and maintainability.
packages/core-generators/src/generators/docker/docker-compose/docker-compose.generator.ts (2)
35-39: LGTM! Consistent refactoring implementation.The dependency declaration, parameter destructuring, and method call have been properly updated to use the new
packageInfoProviderinterface. The logic remains functionally equivalent.
7-7:Details
❓ Verification inconclusive
Verify consistent refactoring across the codebase.
The import change from
projectProvidertopackageInfoProvideraligns with the broader refactoring effort mentioned in the summary.
🏁 Script executed:
#!/bin/bash # Description: Verify that all imports of projectProvider have been updated to packageInfoProvider # Expected: No remaining imports of projectProvider echo "Checking for any remaining projectProvider imports..." rg "import.*projectProvider" --type ts echo "Checking for packageInfoProvider imports..." rg "import.*packageInfoProvider" --type ts -A 2Length of output: 2248
Update remaining
projectProviderimports in testsThe grep results show that the refactoring isn’t complete—there are still imports of
projectProviderin the code-morph test fixtures. Please update these topackageInfoProviderto align with the new provider interface.Files to fix:
- packages/code-morph/src/morphers/tests/build-tasks-as-object/converted/output.ts
- packages/code-morph/src/morphers/tests/build-tasks-as-object/converted/input.ts
[fix_required]
packages/sync/src/templates/extractor-v2/index.ts (1)
1-4: LGTM! Well-structured barrel export for new extractor system.The barrel export appropriately centralizes access to the Template Extractor V2 functionality, providing a clean API surface for configs, runner logic, and core runner implementation.
packages/core-generators/src/generators/node/_composers/node.ts (2)
6-6: LGTM! Proper import of pathRootsGenerator.The import correctly references the new metadata generator that handles path root registration and metadata writing.
26-26: LGTM! Consistent integration pattern.The
pathRootsGeneratoris properly integrated following the established pattern for child generators. This ensures that path root metadata will be generated as part of the node generator composition.packages/project-builder-server/src/template-extractor/index.ts (1)
1-1: LGTM!The export statement is correctly formatted and follows the standard barrel export pattern for making the v2 template extractor available.
packages/utils/src/paths/normalize-path-separators.ts (1)
1-10: LGTM!The implementation correctly normalizes Windows path separators to Unix format using Node.js path constants. The function is well-documented and uses the appropriate
replaceAllmethod for comprehensive replacement.packages/core-generators/src/renderers/raw/index.ts (1)
1-3: LGTM!The barrel export correctly consolidates the raw renderer components (extractor, action, and types) into a unified entry point following established patterns.
packages/core-generators/src/renderers/index.ts (1)
1-3: LGTM!The addition of raw, templates, and text renderer exports correctly expands the available renderer functionality while maintaining the existing TypeScript renderer export. The barrel export pattern is properly implemented.
packages/utils/src/validators/case-validators.ts (1)
27-29: LGTM! Excellent UX improvement with clear error messages.The addition of custom error messages with examples significantly improves the developer experience when validation fails. The messages are consistent, clear, and provide actionable guidance.
Also applies to: 33-35, 39-41, 45-47
.cursor/rules/tests.mdc (1)
1-46: Excellent testing guidelines documentation!This comprehensive guide establishes clear testing patterns and conventions. The memfs example aligns perfectly with the codebase's existing testing patterns, and the organize-act-assert structure promotes maintainable tests.
packages/sync/package.json (1)
23-26: LGTM! Properly structured export for the new extractor-v2 functionality.The export follows the established pattern and correctly exposes both TypeScript declarations and JavaScript modules for the new template extractor v2 system.
packages/utils/src/paths/normalize-path-separators.unit.test.ts (1)
1-17: Well-written test suite with comprehensive coverage.The tests effectively cover both the transformation case (Windows → Unix separators) and the identity case (Unix separators unchanged). The use of
String.rawfor the Windows path is a good practice to avoid double escaping issues.packages/core-generators/src/renderers/templates/plugins/index.ts (1)
1-5: LGTM! Clean barrel export structure.The barrel export pattern is well-implemented and provides a clean API for consuming plugins. The use of
.jsextensions in imports is consistent with ESM module resolution standards.packages/core-generators/src/renderers/text/index.ts (1)
1-4: LGTM! Consistent barrel export pattern.The file follows the same clean barrel export structure as other renderer modules, providing a centralized entry point for text renderer functionality.
packages/react-generators/src/generators/core/react-routes/react-routes.generator.ts (4)
4-4: LGTM! Clean integration with path roots system.The import of
pathRootsProvideris correctly placed with other core generator imports.
43-43: LGTM! Proper dependency declaration.The dependency on
pathRootsProvideris correctly declared in the task dependencies.
49-49: LGTM! Parameter integration follows established pattern.The
pathRootsparameter is properly added to the run method signature.
57-58: LGTM! Well-timed path root registration.The path root registration occurs at the correct point in the flow - after
directoryBaseis computed but before the provider methods are returned. The key name'routes-root'is descriptive and appropriate for the context.packages/fastify-generators/src/generators/core/app-module/app-module.generator.ts (4)
6-6: LGTM! Consistent import integration.The import of
pathRootsProviderfollows the same pattern as other generators integrating with the path roots system.
76-76: LGTM! Proper dependency declaration.The dependency on
pathRootsProvideris correctly declared in the task dependencies.
97-97: LGTM! Parameter integration follows established pattern.The
pathRootsparameter is properly added to the run method signature, maintaining consistency with other generators.
112-112: LGTM! Appropriately placed path root registration.The path root registration occurs at the optimal point in the flow - after
moduleFolderis computed but before child modules are set on the parentappModule. The key name'module-root'is descriptive and appropriate for the context.packages/project-builder-server/src/template-extractor/run-template-extractor.ts (1)
68-70: LGTM! Improved file discovery configuration.The switch from absolute path patterns to relative patterns with
cwdoption is cleaner and more maintainable. Addinggitignore: trueappropriately respects.gitignorerules during file discovery.packages/sync/src/output/generator-task-output.ts (1)
132-137: LGTM! Improved property naming and documentation.The rename from
hasManualIdtoisInstanceis more descriptive and the updated documentation clearly explains when this flag indicates whether a file is an instance of a template or a singleton.packages/sync/src/templates/utils/read-template-file-source.ts (1)
33-38: LGTM! Appropriate migration logic for v2 template system.The conditional path resolution correctly uses
path.isAbsolute()to handle both absolute and relative template paths during the migration to v2. The TODO comment appropriately marks this as temporary migration code.packages/react-generators/src/generators/core/react/react.generator.ts (1)
7-7: LGTM! Consistent migration to packageInfoProvider.The coordinated changes from
projectProvidertopackageInfoProviderare well-executed:
- Import, dependency injection, variable naming, and method calls are all consistently updated
- The method name change from
getProjectName()togetPackageName()better reflects the semantic meaning- Changes align with the broader migration pattern across multiple generators
Also applies to: 114-114, 119-119, 143-143
packages/core-generators/src/renderers/templates/plugins/barrel-import.unit.test.ts (1)
1-106: Comprehensive test coverage for barrel export merging.The test suite provides excellent coverage of the
mergeBarrelExportsfunction with well-structured test cases covering:
- Basic merging and sorting functionality
- Edge cases (empty exports)
- Deduplication of exports from the same module
- Type-only exports with mixed regular exports
- Type-only star exports
The use of inline snapshots ensures consistent output formatting verification.
packages/sync/src/templates/utils/formatter.ts (2)
3-8: Improved import for dynamic parser detection.Good addition of
getFileInfoimport to enable dynamic parser detection instead of maintaining a static whitelist.
53-56: Enhanced formatting logic with dynamic parser detection.Excellent improvement! Replacing the static
ALLOWED_EXTENSIONSwhitelist with Prettier'sgetFileInfoprovides several benefits:
- Automatically supports any file type that Prettier can parse
- Future-proof as new Prettier parsers are added
- Eliminates maintenance overhead of keeping extension lists current
The logic correctly checks if Prettier can infer a parser for the file and returns the original contents unchanged if not.
packages/sync/src/utils/canonical-path.ts (2)
1-1: Appropriate imports for path utilities.Good use of the utility functions for path normalization and POSIX joining from the utils package.
23-39: Well-implemented canonical path conversion function.The
toCanonicalPathfunction is well-designed with:
- Early return for already canonical paths (efficiency)
- Path normalization for cross-platform compatibility
- Comprehensive validation against absolute paths and parent directory references
- Clear error messaging for invalid inputs
- Consistent use of POSIX path joining
The implementation correctly handles the conversion from output-relative paths to canonical format while maintaining security by rejecting potentially dangerous path patterns.
packages/sync/src/templates/extractor-v2/configs/try-create-extractor-json.unit.test.ts (4)
14-45: LGTM! Comprehensive test coverage for the happy path.The test correctly validates that
tryCreateExtractorJsoncreates the expected extractor.json file with proper content when a single generator file is found.
47-68: Good error handling test for missing generator files.The test properly validates that the function throws an appropriate error when no matching generator file is found.
70-94: Excellent edge case coverage for multiple generator files.This test ensures the function fails gracefully when ambiguous generator files exist, preventing incorrect extractor configuration.
96-121: Important formatting validation test.The test verifies proper JSON formatting with indentation and trailing newline, which is crucial for consistent file generation.
packages/fastify-generators/src/generators/core/fastify-scripts/extractor.json (1)
1-14: Well-structured extractor configuration.The JSON configuration correctly defines the fastify-scripts generator with appropriate template metadata:
- Clear naming convention for the generator and template
- Proper raw template type specification
- Correct singleton file options and path root configuration
packages/fastify-generators/src/generators/vitest/prisma-vitest/prisma-vitest.generator.ts (3)
4-4: Provider import updated correctly.The import change from
projectProvidertopackageInfoProvideraligns with the broader refactoring effort mentioned in the AI summary.
87-89: Dependency and parameter updated consistently.The dependency declaration and function parameter have been correctly updated to use
packageInfoinstead ofproject.
101-101: Method call updated appropriately.The call to
getPackageName()instead ofgetProjectName()maintains the same functionality while using the new provider interface.packages/core-generators/src/renderers/text/utils.unit.test.ts (2)
7-14: Excellent regex boundary testing.The test correctly validates that the regex matches standalone identifiers and properly excludes embedded occurrences using word boundaries.
16-21: Good special character handling test.The test validates that special regex characters (like
.) are properly escaped and treated literally in the pattern.packages/sync/src/templates/extractor-v2/runner/index.ts (1)
1-6:Details
✅ Verification successful
Verify that all exported modules exist and don't have naming conflicts.
The barrel file structure looks good and follows common patterns for organizing exports.
Please run this script to verify all exported modules exist:
🏁 Script executed:
#!/bin/bash # Description: Verify that all re-exported modules exist in the runner directory # Check if all the exported files exist fd -t f -e js . packages/sync/src/templates/extractor-v2/runner/ | grep -E "(initialize-template-extractor-plugins|template-extractor-api|template-extractor-context|template-extractor-file-container|template-extractor-plugin|template-file-extractor)\.js$"Length of output: 257
🏁 Script executed:
#!/bin/bash # List TypeScript files in the runner directory to verify matching module names fd -t f -e ts packages/sync/src/templates/extractor-v2/runner/Length of output: 562
🏁 Script executed:
#!/bin/bash # Verify that each re-exported JS module exists in the runner directory base_dir="packages/sync/src/templates/extractor-v2/runner" for module in initialize-template-extractor-plugins template-extractor-api template-extractor-context template-extractor-file-container template-extractor-plugin template-file-extractor; do file="$base_dir/$module.js" if [[ -f "$file" ]]; then echo "FOUND: $module.js" else echo "MISSING: $module.js" fi doneLength of output: 2792
🏁 Script executed:
#!/bin/bash # List all TypeScript files in the runner directory to verify source modules fd -t f -e ts . packages/sync/src/templates/extractor-v2/runnerLength of output: 794
Barrel Exports Verified – all modules accounted for and path extensions are correct
All six re-exports in
packages/sync/src/templates/extractor-v2/runner/index.tsmap to existing source files. In your TypeScript setup (Node ESM resolution), importing with a.jsextension is the intended pattern and will resolve the corresponding.tsmodules at build time.No naming conflicts were detected.
packages/sync/src/templates/extractor-v2/utils/group-template-files-by-type.unit.test.ts (5)
8-20: Excellent use of test helper for reducing duplication.The
createMockEntryhelper function effectively reduces test code duplication while allowing for targeted customization of test data.
22-55: Good test coverage for basic grouping functionality.The test properly verifies that files are grouped by their
typeproperty and validates the expected structure.
90-121: Excellent test for deduplication logic.This test effectively validates the critical business logic of keeping only the latest file for the same generator/template combination, which is essential for the template extraction workflow.
123-156: Comprehensive error handling validation.The multiple assertions ensure the error message contains all relevant information (duplicate names and generator references), which will help with debugging.
189-241: Thorough testing of complex duplicate scenarios.This test validates error handling across multiple types and ensures all duplicate names are reported, demonstrating good edge case coverage.
packages/fastify-generators/src/generators/core/fastify-scripts/fastify-scripts.generator.ts (5)
28-28: Good addition of paths task for separation of concerns.Adding a dedicated paths task aligns well with the new architecture that separates path management from template content.
33-33: Proper integration of paths provider dependency.The addition of the paths provider to the dependencies maintains consistency with the new task structure.
38-38: Function signature updated correctly.The run function signature properly accepts the new paths parameter, maintaining type safety.
57-59: Improved template usage with dynamic paths.The change from raw templates to typed templates with dynamic destination paths is a significant improvement that enhances maintainability and type safety.
11-12:Details
✅ Verification successful
Verify that the new generated imports exist.
The refactoring from raw template constants to separate paths and templates follows a good separation of concerns pattern.
Please run this script to verify the new imports exist:
🏁 Script executed:
#!/bin/bash # Description: Verify that the new generated import files exist # Check if the generated files exist ls -la packages/fastify-generators/src/generators/core/fastify-scripts/generated/template-paths.js ls -la packages/fastify-generators/src/generators/core/fastify-scripts/generated/typed-templates.jsLength of output: 481
🏁 Script executed:
#!/bin/bash # Search entire repository for generated template files (.ts and .js) echo "Looking for 'generated' directories..." find . -type d | grep -i generated || echo "No 'generated' directories found" echo "Searching for template-paths files..." find . -type f | grep -E "template-paths\.(ts|js)" || echo "No template-paths files found" echo "Searching for typed-templates files..." find . -type f | grep -E "typed-templates\.(ts|js)" || echo "No typed-templates files found"Length of output: 6337
Verified Generated Imports
- Source files
template-paths.tsandtyped-templates.tsexist under
packages/fastify-generators/src/generators/core/fastify-scripts/generated.- Importing with a
.jsextension is intentional for ESM after TypeScript compilation.No further action required.
packages/sync/src/templates/extractor-v2/runner/initialize-template-extractor-plugins.unit.test.ts (1)
1-255: Excellent comprehensive test coverage!This test suite thoroughly covers all the important scenarios for plugin initialization:
- Dependency resolution and ordering
- Recursive dependency extraction
- Context population during initialization
- Async plugin support
- Hook registration
- Edge cases (empty plugins, overlapping plugins)
The mock helpers are well-designed and the test assertions are precise. Great work on ensuring the plugin system is thoroughly tested.
packages/project-builder-cli/src/commands/extract-templates.ts (3)
3-3: Integration with V2 extractor looks correct.The import change to use
runTemplateExtractorsForProjectV2aligns with the new Template Extractor V2 infrastructure.
20-24: New auto-generate-extractor option is well-implemented.The option provides useful functionality and has a sensible default value of
true.
25-43: Action function properly updated for V2 integration.The function signature correctly includes the options parameter, and the options object is properly passed to the V2 extractor function.
packages/core-generators/src/renderers/templates/utils/package-path-specifier.ts (1)
19-29: Core resolution logic is correct.The logic for resolving package path specifiers works correctly - returning subpath imports for same-package references and package names for cross-package references.
packages/core-generators/src/renderers/raw/render-raw-template-action.unit.test.ts (2)
16-61: Excellent test coverage for template metadata scenario.The test properly validates file writing with template metadata when
includeTemplateMetadatais enabled. The assertions comprehensively check file content, ID generation, and metadata structure.
63-86: Good coverage for content-based template scenario.The test correctly validates that when using template contents directly, no metadata is attached (as expected when
templateMetadataOptionsis not provided).packages/utils/src/paths/safe-path-join.unit.test.ts (2)
6-153: Excellent comprehensive test coverage for security-critical functionality.This test suite provides outstanding coverage for the
safePathJoinfunction, including:
- Valid path joining scenarios
- Directory traversal attack prevention
- Platform-specific behavior (Windows vs Unix)
- Edge cases with special characters and Unicode
- Proper error messaging validation
The security testing is particularly thorough, covering various directory traversal attack vectors. This is crucial for a security-sensitive utility.
116-121: Good handling of root directory edge case.The test correctly handles the edge case where trying to escape the root directory normalizes to root itself, which is the expected secure behavior.
packages/core-generators/src/renderers/text/utils.ts (1)
40-42:Details
✅ Verification successful
Verify regex lookbehind browser compatibility if used in browser context.
The negative lookbehind
(?<!\\w)requires ES2018 support. If this code runs in browsers, ensure compatibility or provide a fallback.
🏁 Script executed:
#!/bin/bash # Check if this utility is used in browser context echo "Checking for browser/client-side usage of text utils..." rg -A 3 -B 3 "getTextTemplateVariableRegExp|text.*utils" --type js --type tsLength of output: 24355
No browser usage detected—Node‐only context, lookbehind is safe
All occurrences of getTextTemplateVariableRegExp live in generator/sync/server code (Node/CLI), not in browser bundles. Modern Node’s V8 engine supports ES2018 lookbehind, so no fallback is required.packages/core-generators/src/renderers/raw/render-raw-template-action.ts (2)
20-24: LGTM! Excellent use of conditional typing.The conditional typing correctly enforces that instance templates require an
idparameter while singleton templates should not have one. This provides compile-time safety for template usage.
72-72: Good practice: Skip formatting for raw templates.Setting
skipFormatting: trueis appropriate for raw templates since they should be written exactly as provided without any code formatting modifications.packages/sync/src/templates/extractor-v2/runner/template-extractor-file-container.unit.test.ts (2)
7-12: Good mocking pattern following codebase conventions.The use of Vitest automocks for
node:fs/promisescombined with memfs aligns with the established testing patterns in this codebase as noted in the retrieved learnings.
277-283: LGTM! Good test coverage for edge cases.The test correctly verifies that an empty map is returned when no files have been written, ensuring the method behaves consistently across all states.
packages/core-generators/src/generators/node/node/node.generator.ts (2)
137-148: LGTM! Clean transition from project to packageInfo.The new
packageInfotask properly exports the package provider and provides the expected interface methods. The hardcoded paths ('@'and'@/src') are appropriate for package-relative references.
149-161:Details
❓ Verification inconclusive
Verify path root registration integration.
The path roots registration looks correct, but ensure that the
pathRootsProvideris properly initialized and that these specific root names ('package-root','src-root') are documented as part of the public API.
🏁 Script executed:
#!/bin/bash # Description: Verify path roots provider usage and documentation # Search for path root name usage across the codebase echo "=== Searching for path root name usage ===" rg -A 2 -B 2 "'package-root'|'src-root'" --type ts # Look for path roots provider initialization echo -e "\n=== Searching for pathRootsProvider initialization ===" ast-grep --pattern 'pathRootsProvider = $_' # Check for documentation of these path root names echo -e "\n=== Searching for documentation of path root names ===" rg -A 5 -B 5 "package-root|src-root" --type mdLength of output: 1299
Verify provider binding and API documentation
The task registers the
'package-root'and'src-root'keys correctly, but I wasn’t able to find:
- Any explicit initialization or binding of
pathRootsProviderin the DI/container setup.- References to these root names (
'package-root','src-root') in the project’s documentation (e.g., README, API docs).Please:
- Confirm that
pathRootsProvideris registered where this generator runs so it’s injected at runtime.- Document these path-root identifiers in the public API docs (or a user guide) so consumers know to use them.
packages/core-generators/src/providers/project.ts (1)
3-16: LGTM! Well-designed interface expansion.The interface properly expands the previous
ProjectProviderfunctionality with clear method names and documentation. The separation of concerns between package name, root, and source paths is logical.packages/sync/src/templates/metadata/read-template-metadata-files.unit.test.ts (6)
1-13: LGTM: Clean test setup with proper mocking.The test setup correctly mocks both
node:fsandnode:fs/promisesmodules and uses memfs for in-memory filesystem simulation. The beforeEach hook properly resets state between tests.
15-88: LGTM: Comprehensive happy path test.This test thoroughly validates the main functionality by:
- Testing multiple metadata files with different structures
- Verifying file timestamp retrieval and mapping
- Checking proper path resolution and metadata extraction
- Using realistic test data that matches expected schemas
The assertions are well-structured and verify all key aspects of the returned data.
103-123: LGTM: Proper error handling for missing source files.The test correctly verifies that the function throws an appropriate error when a metadata file references a source file that doesn't exist. The error message is descriptive and includes the problematic file path.
135-155: LGTM: Schema validation testing.The test properly validates that invalid metadata (using PascalCase instead of kebab-case for the name field) is rejected with an appropriate error message. This ensures the schema validation is working correctly.
157-196: LGTM: Nested directory structure handling.This test ensures the function can discover and process metadata files in deeply nested directory structures, which is essential for real-world project layouts.
198-236: LGTM: Multiple files in single metadata file.The test validates that a single metadata file can contain references to multiple source files, including files in subdirectories, which is a common use case.
packages/core-generators/src/renderers/text/render-text-template-group-action.ts (3)
13-21: LGTM: Well-designed conditional type system.The type definitions correctly implement conditional logic to:
- Determine if templates have variables using
HasVariables<T>- Filter template groups to only include templates that require variables
- Create properly typed variable requirements per template
This ensures type safety while allowing flexibility for templates with and without variables.
33-38: LGTM: Sophisticated conditional variable requirements.The input type correctly makes the
variablesproperty:
- Optional when no templates in the group require variables
- Required when at least one template requires variables
This provides excellent developer experience with accurate TypeScript intellisense.
67-72: LGTM: Proper error handling with context.The error handling correctly catches exceptions from individual template rendering and enhances them with context indicating which template failed. This will greatly improve debugging when working with template groups.
packages/sync/src/templates/extractor-v2/utils/update-extractor-template-entries.ts (4)
8-19: LGTM: Clear function documentation.The JSDoc comment clearly explains the function's purpose, parameters, and error conditions. The type annotations provide good type safety.
20-32: LGTM: Proper error handling for missing configurations.The function correctly validates that extractor configurations exist for each generator and throws descriptive errors when they don't. This prevents silent failures that could be difficult to debug.
52-68: LGTM: Clean configuration update and persistence.The configuration update correctly:
- Preserves all existing configuration properties using destructuring
- Sorts template keys for consistent output
- Writes the updated configuration to disk with pretty formatting
- Updates the cached configuration for immediate availability
The use of
sortObjectKeysandstringifyPrettyCompactensures readable and consistent file output.
42-50:Details
❓ Verification inconclusive
Verify the deduplication strategy.
The deduplication logic removes existing templates that have the same
nameas new templates, but templates are keyed bygeneratorTemplatePath. This could potentially remove templates with different paths but the same name.Please verify this is the intended behavior. Should the deduplication be based on:
- Template name (current implementation) - removes any existing template with same name regardless of path
- Template path - removes only if exact path matches
- Both name and path - more complex logic
Run this script to check how this deduplication is used elsewhere:
🏁 Script executed:
#!/bin/bash # Description: Check how template deduplication is expected to work in the codebase # Search for similar deduplication patterns rg -A 5 -B 5 "newTemplateNames|duplicat" --type ts # Search for template name vs path usage patterns rg -A 3 -B 3 "templateConfig\.name|generatorTemplatePath" --type tsLength of output: 66921
Verify deduplication in update-extractor-template-entries.ts
The code in
packages/sync/src/templates/extractor-v2/utils/update-extractor-template-entries.ts(around lines 42–50) does this:// Remove any existing templates that have the same name as new templates // This prevents duplicates when template names match but paths differ const newTemplateNames = new Set(entries.map((e) => e.metadata.name)); const filteredExistingTemplates = Object.fromEntries( Object.entries(templates).filter( ([, templateConfig]) => !newTemplateNames.has(templateConfig.name), ), );Here,
templatesis keyed bygeneratorTemplatePath, yet the filter only checkstemplateConfig.name. This will drop any existing template whose name matches a new entry, even if its path is different.Is this the intended behavior? Should we instead:
- Continue deduping by name (current)
- Dedup by path only
- Require matching both name and path
Please confirm the desired strategy.
packages/sync/src/templates/metadata/read-template-metadata-files.ts (4)
16-20: LGTM: Well-defined interface.The
TemplateMetadataFileEntryinterface clearly defines the structure of returned data with absolute path, metadata, and modification time - all essential for template processing.
31-40: LGTM: Proper file discovery configuration.The globby configuration correctly:
- Uses absolute paths for consistent path handling
- Respects
.gitignorefiles to avoid processing ignored files- Restricts to files only (no directories)
- Uses the filesystem adapter for compatibility with mocking in tests
This ensures reliable file discovery across different project structures.
44-47: LGTM: Robust schema validation.The schema validation uses
passthrough()to allow additional properties while validating the required base schema. This provides flexibility for future extensions while ensuring core requirements are met.
52-60: LGTM: Proper error handling for missing source files.The error handling correctly:
- Uses
handleFileNotFoundErrorto distinguish file not found from other errors- Provides descriptive error messages including both the missing filename and the metadata file path
- Fails fast when referenced source files don't exist
This prevents processing of incomplete or invalid metadata.
packages/sync/src/templates/extractor-v2/utils/update-extractor-template-entries.unit.test.ts (2)
486-500: Good defensive programming!Excellent test case for handling empty metadata entries. This ensures the function handles edge cases gracefully without throwing errors.
90-91:Details
✅ Verification successful
Verify if
updateExtractorTemplateEntriesis synchronous.The function is called without
await, which suggests it's synchronous. However, if it performs any async operations internally, this could lead to race conditions in tests.Let me check the implementation of
updateExtractorTemplateEntries:
🏁 Script executed:
#!/bin/bash # Check if updateExtractorTemplateEntries is async ast-grep --pattern 'function updateExtractorTemplateEntries' packages/sync/src/templates/extractor-v2/utils/update-extractor-template-entries.ts ast-grep --pattern 'export $_ function updateExtractorTemplateEntries' packages/sync/src/templates/extractor-v2/utils/update-extractor-template-entries.ts ast-grep --pattern 'export const updateExtractorTemplateEntries = $_' packages/sync/src/templates/extractor-v2/utils/update-extractor-template-entries.tsLength of output: 6943
No async operations detected in updateExtractorTemplateEntries
The function is declared asexport function updateExtractorTemplateEntries(...): void { … }and returns
voidwithout anyasynckeyword or internal asynchronous calls. Calling it withoutawaitis correct.packages/sync/src/templates/extractor-v2/runner/initialize-template-extractor-plugins.ts (1)
32-77: Well-structured plugin initialization with proper dependency handling!The implementation correctly handles:
- Recursive plugin extraction
- Topological sorting for dependency order
- Sequential initialization with isolated contexts
- Hook collection and registration
The use of topological sort ensures plugins are initialized in the correct order respecting their dependencies.
packages/core-generators/src/renderers/text/render-text-template-file-action.unit.test.ts (2)
94-100:generatorInfomissing – test relies on implicit defaultThe expectation
expect(file?.id).toBe('test-generator:test-id');assumes
builder.generatorInfo.name === 'test-generator', yet nogeneratorInfois passed totestActionin this case.
Provide it explicitly to make the test self-contained and prevent breakage iftestAction’s default ever changes.-const output = await testAction(action); +const output = await testAction(action, { + generatorInfo: { name: 'test-generator', baseDirectory: '' }, +});
8-10: 🛠️ Refactor suggestionGlobal
fsmocks leak across test suites
vi.mock('fs');andvi.mock('fs/promises');are declared at module-scope, so the mocks stay active for every other test file that runs after this one.
Couple of options:-vi.mock('fs'); -vi.mock('fs/promises'); +const unmockFs = () => { + vi.unmock('fs'); + vi.unmock('fs/promises'); +}; + +beforeAll(() => { + vi.mock('fs'); + vi.mock('fs/promises'); +}); + +afterAll(unmockFs);This confines the mock to this suite and avoids surprising side-effects elsewhere.
⛔ Skipped due to learnings
Learnt from: kingston PR: halfdomelabs/baseplate#510 File: packages/project-builder-server/src/sync/conflict-file-monitor.test.ts:19-24 Timestamp: 2025-04-23T06:44:30.952Z Learning: In this codebase, when testing file operations, Vitest's automocks for 'node:fs' and 'node:fs/promises' are used in conjunction with memfs, but without replacing the mock implementation explicitly in each test file. The virtual filesystem is populated using vol.fromJSON or similar methods, which works seamlessly with the automocks.Learnt from: kingston PR: halfdomelabs/baseplate#510 File: packages/project-builder-server/src/sync/conflict-file-monitor.test.ts:19-24 Timestamp: 2025-04-23T06:44:30.952Z Learning: When testing file operations in this codebase, the pattern is to use Vitest automocks for 'node:fs' and 'node:fs/promises' (without explicit implementation replacement) while populating a virtual filesystem with vol.fromJSON() from memfs. File operations in tests are performed directly via vol.promises methods.Learnt from: kingston PR: halfdomelabs/baseplate#510 File: packages/project-builder-server/src/sync/conflict-file-monitor.test.ts:19-24 Timestamp: 2025-04-23T06:44:30.952Z Learning: In the project-builder-server test suite, Vitest automocks for 'node:fs' and 'node:fs/promises' are already configured to use memfs without needing explicit implementation replacement in each test file.packages/sync/src/templates/extractor-v2/configs/try-create-extractor-json.ts (1)
73-75: Wrong value written tonamefield
extractorConfig.namecurrently receivesgeneratorPath, dropping the package scope (packageName).
Down-stream look-ups typically expect the full generator name (${packageName}:${generatorPath}).-const extractorConfig = { - name: generatorPath, -}; +const extractorConfig = { name: generatorName };packages/sync/src/templates/extractor-v2/runner/template-extractor-file-container.ts (1)
39-42: LGTM: Conditional formatting logic is well-implemented.The type checking and conditional formatting for string vs Buffer contents is handled correctly, ensuring only string contents are formatted while preserving binary data.
packages/sync/src/templates/metadata/metadata.ts (4)
1-1: LGTM: Improved validation with standardized validators.Using
CASE_VALIDATORS.KEBAB_CASEinstead of manual regex provides better consistency and error messages across the codebase.
6-8: Clear documentation improvement for the name field.The updated comment clearly specifies that the name must be unique within a generator, which is important for understanding the scope of uniqueness.
16-16: Good validation enhancement for generator field.Adding
.min(1)ensures the generator field cannot be empty, which is a sensible constraint for required metadata.
17-21: Well-documented migration plan with TODO comment.The TODO comment with a specific date provides clear guidance for when this field should be removed after the v2 migration is complete.
packages/fastify-generators/src/generators/core/readme/readme.generator.ts (5)
2-4: LGTM: Migration to new provider system.The transition from
projectProvidertopackageInfoProvideraligns with the broader v2 migration and provides a more focused interface for package information.
8-9: Good adoption of typed templates and paths.Using generated typed templates and template paths provides better type safety and IDE support compared to string literals.
20-24: Well-structured task dependencies.The explicit dependency on both
packageInfoandpathsproviders makes the generator's requirements clear and enables proper dependency injection.
27-28: Improved fallback logic for project name.The fallback from
descriptor.projectNametopackageInfo.getPackageName()provides a sensible default while maintaining flexibility.
34-35: Enhanced path handling with dynamic resolution.Using
paths.readmeinstead of a hardcoded string enables flexible path configuration and better integration with the template paths system.packages/core-generators/src/generators/metadata/path-roots/path-roots.generator.ts (3)
16-24: Well-designed provider interface.The
PathRootsProviderinterface is clean and focused, with clear documentation of the method's purpose and parameters.
48-60: Good duplicate prevention logic.The validation against duplicate canonical paths prevents configuration conflicts and provides a clear error message.
67-71: LGTM: Clean metadata file writing.The conditional metadata writing with pretty-formatted JSON is well-implemented and respects the metadata inclusion settings.
packages/sync/src/templates/extractor-v2/configs/template-extractor-config-lookup.unit.test.ts (1)
9-10:⚠️ Potential issue
fsis mocked but never wired to memfs – tests will operate on empty stubs
vi.mock('node:fs')andvi.mock('node:fs/promises')without a factory cause Vitest to auto-mock those modules with empty objects. Consequently, any real I/O used byTemplateExtractorConfigLookupwill silently fail or return undefined, potentially masking bugs.Wire the mocks to
memfsso the lookup actually sees the in-memory filesystem:vi.mock('node:fs', () => ({ default: vol as unknown })); vi.mock('node:fs/promises', () => ({ default: vol.promises }));(or use
memfs/lib/fshelpers).
This ensures the tests exercise real fs semantics instead of no-op stubs.⛔ Skipped due to learnings
Learnt from: kingston PR: halfdomelabs/baseplate#510 File: packages/project-builder-server/src/sync/conflict-file-monitor.test.ts:19-24 Timestamp: 2025-04-23T06:44:30.952Z Learning: In this codebase, when testing file operations, Vitest's automocks for 'node:fs' and 'node:fs/promises' are used in conjunction with memfs, but without replacing the mock implementation explicitly in each test file. The virtual filesystem is populated using vol.fromJSON or similar methods, which works seamlessly with the automocks.Learnt from: kingston PR: halfdomelabs/baseplate#510 File: packages/project-builder-server/src/sync/conflict-file-monitor.test.ts:19-24 Timestamp: 2025-04-23T06:44:30.952Z Learning: When testing file operations in this codebase, the pattern is to use Vitest automocks for 'node:fs' and 'node:fs/promises' (without explicit implementation replacement) while populating a virtual filesystem with vol.fromJSON() from memfs. File operations in tests are performed directly via vol.promises methods.Learnt from: kingston PR: halfdomelabs/baseplate#510 File: packages/project-builder-server/src/sync/conflict-file-monitor.test.ts:19-24 Timestamp: 2025-04-23T06:44:30.952Z Learning: In the project-builder-server test suite, Vitest automocks for 'node:fs' and 'node:fs/promises' are already configured to use memfs without needing explicit implementation replacement in each test file.packages/core-generators/src/renderers/templates/plugins/template-paths/paths-file.ts (1)
94-103: Single-root assumption may break for paths containing multiple placeholders
PATH_ROOT_REGEXcaptures only the first{root}at the start of the string. If a relative path can legitimately contain several placeholders (e.g.{feature}/{entity}/index.ts), the loop records only the first and later substitution will fail.Verify that multi-placeholder paths are impossible or broaden the extraction logic:
-const pathRootMatches = PATH_ROOT_REGEX.exec(pathRootRelativePath); +const pathRootMatches = [...pathRootRelativePath.matchAll(PATH_ROOT_REGEX)]; if (!pathRootMatches.length) throw …; pathRootMatches.forEach(([, root]) => pathRootsUsed.add(root));This prevents false negatives when additional placeholders appear.
packages/sync/src/templates/extractor-v2/configs/extractor-config.schema.ts (1)
1-18: LGTM! Well-structured schema for template configuration.The schema properly validates template names using kebab-case and allows extensibility through
.passthrough().packages/core-generators/src/renderers/raw/raw-template-file-extractor.ts (1)
98-98:⚠️ Potential issuePotential code injection risk in generated TypeScript code.
Using
JSON.stringifydirectly in template literals can lead to code injection ifconfig.fileOptionscontains special characters or malicious content. Consider using a safer approach or ensuring proper escaping.- fileOptions: ${JSON.stringify(config.fileOptions)}, + fileOptions: ${JSON.stringify(config.fileOptions, null, 2) + .split('\n') + .map((line, index) => index === 0 ? line : ' ' + line) + .join('\n')},Additionally, consider validating
config.fileOptionsagainst a schema before using it in code generation to ensure it only contains expected properties.Likely an incorrect or invalid review comment.
packages/sync/src/templates/extractor-v2/runner/template-extractor-plugin.ts (1)
1-53: LGTM! Well-structured plugin system.The plugin interface is well-designed with proper typing for dependencies and lifecycle hooks. The use of generics ensures type safety across plugin dependencies.
packages/sync/src/templates/extractor-v2/configs/template-extractor-config-lookup.ts (3)
149-165: Good error aggregation patternThe initialization method correctly collects all errors and reports them together, which provides better user experience by showing all package indexing failures at once rather than failing on the first error.
6-6:Details
✅ Verification successful
Consider cross-platform path handling implications
Using
node:path/posixexplicitly forces POSIX-style paths. While this ensures consistent path separators across platforms, it may cause issues on Windows systems if the paths are used with native file system operations elsewhere in the codebase.
🏁 Script executed:
#!/bin/bash # Description: Check if there are any native path operations that might conflict with POSIX paths # Search for uses of native path module rg -A 3 "from 'node:path'" --type ts | grep -v posix # Search for path.sep usage which would indicate platform-specific path handling rg "path\.sep" --type tsLength of output: 42310
🏁 Script executed:
#!/bin/bash sed -n '1,200p' packages/sync/src/templates/extractor-v2/configs/template-extractor-config-lookup.tsLength of output: 5636
No action needed – POSIX-style paths are safe here
This file deliberately uses
node:path/posixto generate forward-slash patterns forglobbyand consistent config lookups. On Windows, Node’sfsAPIs accept POSIX-style separators transparently, andreadJsonWithSchemawill correctly read paths likeC:/…/extractor.json.No conflicting native-path usage or
path.sepchecks occur in this module, so forcing POSIX semantics actually improves cross-platform consistency without breaking Windows file operations.
86-89:Details
✅ Verification successful
Potential edge case in relative path calculation
The relative path calculation assumes the provider directory is always within the package path. If the directory structure is unusual or symlinks are involved, this could produce unexpected results.
🏁 Script executed:
#!/bin/bash # Description: Check if there are any symlinks or unusual directory structures in provider configs # Find all providers.json files and check their parent directories fd "providers.json" --type f --exec dirname {} \; | sort | uniq | while read -r dir; do if [ -L "$dir" ]; then echo "Symlink found: $dir -> $(readlink -f "$dir")" fi doneLength of output: 148
I’d like to inspect how
packagePathandproviderDirectoryare derived around that snippet to confirm whether a directory could sit outside the package root. Let me dump the surrounding context:
🏁 Script executed:
#!/bin/bash set -eo pipefail # Locate the file file=$(fd "template-extractor-config-lookup.ts" --type f) echo "File: $file" # Show top of file for definitions/imports sed -n '1,200p' "$file" echo "----" # Show where packagePath is set rg -n "packagePath" "$file" echo "----" # Show where providerDirectory is set rg -n "providerDirectory" "$file"Length of output: 7135
No change required for relative path calculation
Theglobbycall is scoped topackagePath(using patterns likepath.join(packagePath, '**/providers.json')), so everyproviderDirectorywill always reside under the package root. Therefore,path.relative(packagePath, providerDirectory)cannot produce “up-level” paths and is safe as written.
| { | ||
| "project.ts": { | ||
| "packageInfoProvider": { | ||
| "type": "path", | ||
| "pathRoots": [ | ||
| { | ||
| "name": "package-root", | ||
| "method": "getPackageRoot" | ||
| }, | ||
| { | ||
| "name": "src-root", | ||
| "method": "getPackageSrcPath" | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM! Well-structured provider configuration.
The JSON configuration properly defines the packageInfoProvider with appropriate path roots and methods. The structure aligns with the migration from projectProvider to packageInfoProvider mentioned in the broader refactoring.
Verify the referenced methods exist and the provider migration is complete:
🏁 Script executed:
#!/bin/bash
# Description: Verify the provider methods exist and migration is complete
# Check for getPackageRoot and getPackageSrcPath method definitions
rg -A 5 "getPackageRoot|getPackageSrcPath"
# Check if any old projectProvider references remain
rg "projectProvider" --type ts --type jsLength of output: 4886
Verify provider migration and update remaining projectProvider references
All referenced methods (getPackageRoot, getPackageSrcPath) are present and correctly wired, but there are still leftover imports of projectProvider in the codebase. Please update or remove these to complete the migration:
• packages/code-morph/src/morphers/tests/build-tasks-as-object/converted/input.ts
• packages/code-morph/src/morphers/tests/build-tasks-as-object/converted/output.ts
Once these references are replaced with the new packageInfoProvider (or otherwise reconciled), the migration will be complete.
🤖 Prompt for AI Agents
In packages/core-generators/src/providers/providers.json lines 1 to 17, the JSON
correctly configures the packageInfoProvider with the required methods. However,
to complete the migration from projectProvider to packageInfoProvider, locate
and update or remove all remaining imports and references to projectProvider in
the codebase, specifically in
packages/code-morph/src/morphers/tests/build-tasks-as-object/converted/input.ts
and output.ts. Replace these with packageInfoProvider or adjust accordingly to
finalize the migration.
packages/core-generators/src/renderers/schemas/template-file-options.ts
Outdated
Show resolved
Hide resolved
packages/core-generators/src/renderers/templates/utils/package-path-specifier.ts
Show resolved
Hide resolved
| export const packageInfoProvider = | ||
| createReadOnlyProviderType<PackageInfoProvider>('package'); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the provider name change impact across the codebase.
The provider name changed from 'project' to 'package'. Ensure all references to the old provider name have been updated consistently.
🏁 Script executed:
#!/bin/bash
# Description: Verify provider name migration from 'project' to 'package'
# Search for any remaining references to the old provider name
echo "=== Searching for old 'project' provider references ==="
rg -A 2 -B 2 "createProviderType.*'project'|createReadOnlyProviderType.*'project'" --type ts
# Search for old projectProvider usage
echo -e "\n=== Searching for old projectProvider usage ==="
rg "projectProvider" --type ts
# Verify new packageInfoProvider usage
echo -e "\n=== Verifying new packageInfoProvider usage ==="
rg -A 1 "packageInfoProvider" --type tsLength of output: 5554
Remaining projectProvider references in tests must be updated
The migration to 'package' was applied across the generators, but two test fixtures still import and use the old projectProvider. Update these to use packageInfoProvider (and adjust export calls as needed).
• packages/code-morph/src/morphers/tests/build-tasks-as-object/converted/input.ts
• packages/code-morph/src/morphers/tests/build-tasks-as-object/converted/output.ts
🤖 Prompt for AI Agents
In packages/core-generators/src/providers/project.ts at lines 18-19, the
provider name was changed from 'project' to 'package'. However, there are still
references to the old 'projectProvider' in test fixtures located in
packages/code-morph/src/morphers/tests/build-tasks-as-object/converted/input.ts
and output.ts. Update these test files to replace all imports and usages of
'projectProvider' with 'packageInfoProvider', and adjust any related export
calls accordingly to ensure consistency with the new provider name.
| api.writeTemplateFile( | ||
| file.generator, | ||
| file.generatorTemplatePath, | ||
| templateContents, | ||
| ); |
There was a problem hiding this comment.
writeTemplateFile invocation is not awaited – race with commit() is likely
api.writeTemplateFile is presumably asynchronous (similar helpers in this package usually buffer writes).
Because the promise is not awaited, limit’s callback resolves immediately and fileContainer.commit() (triggered in the runner) can execute before all template writes finish, leading to partial or empty output.
- api.writeTemplateFile(
+ await api.writeTemplateFile(
file.generator,
file.generatorTemplatePath,
templateContents,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| api.writeTemplateFile( | |
| file.generator, | |
| file.generatorTemplatePath, | |
| templateContents, | |
| ); | |
| await api.writeTemplateFile( | |
| file.generator, | |
| file.generatorTemplatePath, | |
| templateContents, | |
| ); |
🤖 Prompt for AI Agents
In packages/core-generators/src/renderers/text/text-template-file-extractor.ts
around lines 96 to 100, the call to api.writeTemplateFile is asynchronous but
not awaited, causing a race condition with fileContainer.commit(). To fix this,
add an await before api.writeTemplateFile to ensure the write completes before
proceeding, preventing partial or empty output.
packages/core-generators/src/renderers/text/text-template-file-extractor.ts
Show resolved
Hide resolved
| export function getGeneratedTemplateConstantName( | ||
| generatorName: string, | ||
| suffix: string, | ||
| ): string { | ||
| return `${constantCase(generatorName.split('#')[1])}_${constantCase(suffix)}`; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the export name for a generated template file for a particular generator | ||
| * e.g. `core/react-routes` + `templates` -> `coreReactRoutesTemplates` | ||
| * | ||
| * @param generatorName - The name of the generator. | ||
| * @param suffix - The suffix of the export name. | ||
| * @returns The export name for the generated template file. | ||
| */ | ||
| export function getGeneratedTemplateExportName( | ||
| generatorName: string, | ||
| suffix: string, | ||
| ): string { | ||
| return `${camelCase(generatorName.split('#')[1])}${pascalCase(suffix)}`; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the interface name for a generated template file for a particular generator | ||
| * e.g. `core/react-routes` + `templates` -> `CoreReactRoutesTemplates` | ||
| * | ||
| * @param generatorName - The name of the generator. | ||
| * @param suffix - The suffix of the interface name. | ||
| * @returns The interface name for the generated template file. | ||
| */ | ||
| export function getGeneratedTemplateInterfaceName( | ||
| generatorName: string, | ||
| suffix: string, | ||
| ): string { | ||
| return `${pascalCase(generatorName.split('#')[1])}${pascalCase(suffix)}`; | ||
| } | ||
|
|
||
| export function getGeneratedTemplateProviderName( | ||
| generatorName: string, | ||
| suffix: string, | ||
| ): string { | ||
| return `${kebabCase(generatorName.split('#')[1])}-${kebabCase(suffix)}`; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
generatorName.split('#')[1] is unsafe – generator names without # break all helpers
All four helpers assume a scope#name format; if the # is absent (or doubled) split('#')[1] is undefined, producing "UNDEFINED" or throwing when passed to change-case.
Refactor once and reuse:
- return `${constantCase(generatorName.split('#')[1])}_${constantCase(suffix)}`;
+ const namePart = generatorName.includes('#')
+ ? generatorName.split('#')[1] // after '#'
+ : generatorName; // fall back to whole string
+ return `${constantCase(namePart)}_${constantCase(suffix)}`;Apply the same guard in getGeneratedTemplateExportName, getGeneratedTemplateInterfaceName, and getGeneratedTemplateProviderName (DRY: extract getNamePart()).
🤖 Prompt for AI Agents
In
packages/core-generators/src/renderers/templates/utils/generated-template-file-names.ts
between lines 11 and 53, the code unsafely uses generatorName.split('#')[1]
assuming the '#' character is always present, which can cause undefined values
and errors. Refactor by extracting a helper function getNamePart() that safely
returns the part after '#' if present or the whole generatorName if not. Replace
all occurrences of generatorName.split('#')[1] in
getGeneratedTemplateConstantName, getGeneratedTemplateExportName,
getGeneratedTemplateInterfaceName, and getGeneratedTemplateProviderName with
calls to this new helper to ensure safe and consistent behavior.
| const textTemplateFileVariableSchema = z.object({ | ||
| // The description of the variable. | ||
| description: z.string().optional(), | ||
| }); | ||
|
|
||
| export const textTemplateGeneratorTemplateMetadataSchema = | ||
| templateConfigSchema.extend({ | ||
| /** | ||
| * The options for the template file | ||
| */ | ||
| fileOptions: templateFileOptionsSchema, | ||
| /** | ||
| * The path of the template relative to the closest file path root. | ||
| */ | ||
| pathRootRelativePath: z.string().optional(), | ||
| /** | ||
| * The group to assign the template to when generating the typed templates. | ||
| */ | ||
| group: z.string().optional(), | ||
| /** | ||
| * The variables for the template. | ||
| */ | ||
| variables: z.record(z.string(), textTemplateFileVariableSchema).optional(), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding validation for variable names.
Variable names will likely be used in template substitution. Consider adding validation to prevent potential injection attacks or invalid variable names.
const textTemplateFileVariableSchema = z.object({
// The description of the variable.
description: z.string().optional(),
});
+const VALID_VARIABLE_NAME = z.string().regex(
+ /^[a-zA-Z_][a-zA-Z0-9_]*$/,
+ 'Variable name must start with a letter or underscore and contain only alphanumeric characters and underscores'
+);
+
export const textTemplateGeneratorTemplateMetadataSchema =
templateConfigSchema.extend({
/**
* The options for the template file
*/
fileOptions: templateFileOptionsSchema,
/**
* The path of the template relative to the closest file path root.
*/
pathRootRelativePath: z.string().optional(),
/**
* The group to assign the template to when generating the typed templates.
*/
group: z.string().optional(),
/**
* The variables for the template.
*/
- variables: z.record(z.string(), textTemplateFileVariableSchema).optional(),
+ variables: z.record(VALID_VARIABLE_NAME, textTemplateFileVariableSchema).optional(),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const textTemplateFileVariableSchema = z.object({ | |
| // The description of the variable. | |
| description: z.string().optional(), | |
| }); | |
| export const textTemplateGeneratorTemplateMetadataSchema = | |
| templateConfigSchema.extend({ | |
| /** | |
| * The options for the template file | |
| */ | |
| fileOptions: templateFileOptionsSchema, | |
| /** | |
| * The path of the template relative to the closest file path root. | |
| */ | |
| pathRootRelativePath: z.string().optional(), | |
| /** | |
| * The group to assign the template to when generating the typed templates. | |
| */ | |
| group: z.string().optional(), | |
| /** | |
| * The variables for the template. | |
| */ | |
| variables: z.record(z.string(), textTemplateFileVariableSchema).optional(), | |
| }); | |
| const textTemplateFileVariableSchema = z.object({ | |
| // The description of the variable. | |
| description: z.string().optional(), | |
| }); | |
| const VALID_VARIABLE_NAME = z.string().regex( | |
| /^[a-zA-Z_][a-zA-Z0-9_]*$/, | |
| 'Variable name must start with a letter or underscore and contain only alphanumeric characters and underscores' | |
| ); | |
| export const textTemplateGeneratorTemplateMetadataSchema = | |
| templateConfigSchema.extend({ | |
| /** | |
| * The options for the template file | |
| */ | |
| fileOptions: templateFileOptionsSchema, | |
| /** | |
| * The path of the template relative to the closest file path root. | |
| */ | |
| pathRootRelativePath: z.string().optional(), | |
| /** | |
| * The group to assign the template to when generating the typed templates. | |
| */ | |
| group: z.string().optional(), | |
| /** | |
| * The variables for the template. | |
| */ | |
| variables: z.record(VALID_VARIABLE_NAME, textTemplateFileVariableSchema).optional(), | |
| }); |
🤖 Prompt for AI Agents
In packages/core-generators/src/renderers/text/types.ts between lines 13 and 36,
the schema for template variables lacks validation for variable names, which
could lead to injection attacks or invalid names during template substitution.
Add a validation step to ensure variable names conform to a safe pattern, such
as allowing only alphanumeric characters and underscores, by applying a regex or
custom validation function to the keys of the variables record in the
textTemplateGeneratorTemplateMetadataSchema.
| * The TemplateFileExtractor implements the core logic for extracting template | ||
| * metadata from source files. | ||
| * | ||
| * It is responsible for: | ||
| * - Extracting metadata from source files | ||
| * - Writing metadata to the `extractor.json` file | ||
| * - Writing template files to the package | ||
| */ | ||
| export interface TemplateFileExtractor< | ||
| TGeneratorTemplateMetadata extends z.ZodSchema = typeof templateConfigSchema, | ||
| TOutputTemplateMetadata extends | ||
| z.ZodSchema = typeof templateFileMetadataBaseSchema, | ||
| TExtractorConfig extends z.ZodSchema = z.ZodUnknown, | ||
| TPluginDependencies extends | ||
| TemplateExtractorPluginDependencies = TemplateExtractorPluginDependencies, | ||
| > { | ||
| /** | ||
| * The name of the extractor. | ||
| */ | ||
| name: string; | ||
| /** | ||
| * The schema for the metadata for a template in the `extractor.json` file. | ||
| */ | ||
| generatorTemplateMetadataSchema?: TGeneratorTemplateMetadata; | ||
| /** | ||
| * The schema for the metadata for a generated file in the `.template-metadata.json` file. | ||
| */ | ||
| outputTemplateMetadataSchema?: TOutputTemplateMetadata; | ||
| /** | ||
| * The schema for the config for the extractor in the `extractor.json` file. | ||
| */ | ||
| extractorConfigSchema?: TExtractorConfig; | ||
| /** | ||
| * The plugins to use for the extractor. | ||
| */ | ||
| pluginDependencies?: TPluginDependencies; | ||
|
|
||
| /** | ||
| * Extracts the metadata entries for the given files. | ||
| * | ||
| * @param files - The files to extract metadata from. | ||
| * @param context - The context for the extractor. | ||
| */ | ||
| extractTemplateMetadataEntries( | ||
| files: TemplateFileExtractorSourceFile<z.infer<TOutputTemplateMetadata>>[], | ||
| context: TemplateExtractorContext<TPluginDependencies>, | ||
| ): | ||
| | Promise< | ||
| TemplateFileExtractorMetadataEntry< | ||
| z.infer<TGeneratorTemplateMetadata> | ||
| >[] | ||
| > | ||
| | TemplateFileExtractorMetadataEntry<z.infer<TGeneratorTemplateMetadata>>[]; | ||
|
|
||
| /** | ||
| * Writes the files in the metadata entries to the generator's templates directory converting | ||
| * any written templates to their templatized form. | ||
| * | ||
| * @param metadataEntries - The metadata entries to write. | ||
| * @param context - The context for the extractor. | ||
| * @param api - The API for the extractor. | ||
| */ | ||
| writeTemplateFiles( | ||
| metadataEntries: TemplateFileExtractorMetadataEntry< | ||
| z.infer<TGeneratorTemplateMetadata> | ||
| >[], | ||
| context: TemplateExtractorContext<TPluginDependencies>, | ||
| api: TemplateExtractorApi, | ||
| ): Promise<void>; | ||
|
|
||
| /** | ||
| * Writes the files in the metadata entries to the generator's generated directory creating | ||
| * the necessary typed template files. | ||
| * | ||
| * @param generatorNames - The names of the generators to write. | ||
| * @param context - The context for the extractor. | ||
| * @param api - The API for the extractor. | ||
| */ | ||
| writeGeneratedFiles( | ||
| generatorNames: string[], | ||
| context: TemplateExtractorContext<TPluginDependencies>, | ||
| api: TemplateExtractorApi, | ||
| ): Promise<void> | void; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider documenting the extractor lifecycle.
The interface defines multiple methods but doesn't clearly document the expected order of operations or lifecycle. Consider adding documentation about:
- When
extractTemplateMetadataEntriesis called - The relationship between extracted metadata and
writeTemplateFiles - When
writeGeneratedFilesis called in the process
Add comprehensive documentation above the interface:
/**
* The TemplateFileExtractor implements the core logic for extracting template
* metadata from source files.
*
* It is responsible for:
* - Extracting metadata from source files
* - Writing metadata to the `extractor.json` file
* - Writing template files to the package
+ *
+ * ## Lifecycle
+ * 1. `extractTemplateMetadataEntries` - Called first to extract metadata from source files
+ * 2. `writeTemplateFiles` - Called to write templatized versions of files to generator's templates directory
+ * 3. Plugin hooks are executed (afterExtract)
+ * 4. `writeGeneratedFiles` - Called to write typed template files to generator's generated directory
+ * 5. Plugin hooks are executed (afterWrite in reverse dependency order)
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * The TemplateFileExtractor implements the core logic for extracting template | |
| * metadata from source files. | |
| * | |
| * It is responsible for: | |
| * - Extracting metadata from source files | |
| * - Writing metadata to the `extractor.json` file | |
| * - Writing template files to the package | |
| */ | |
| export interface TemplateFileExtractor< | |
| TGeneratorTemplateMetadata extends z.ZodSchema = typeof templateConfigSchema, | |
| TOutputTemplateMetadata extends | |
| z.ZodSchema = typeof templateFileMetadataBaseSchema, | |
| TExtractorConfig extends z.ZodSchema = z.ZodUnknown, | |
| TPluginDependencies extends | |
| TemplateExtractorPluginDependencies = TemplateExtractorPluginDependencies, | |
| > { | |
| /** | |
| * The name of the extractor. | |
| */ | |
| name: string; | |
| /** | |
| * The schema for the metadata for a template in the `extractor.json` file. | |
| */ | |
| generatorTemplateMetadataSchema?: TGeneratorTemplateMetadata; | |
| /** | |
| * The schema for the metadata for a generated file in the `.template-metadata.json` file. | |
| */ | |
| outputTemplateMetadataSchema?: TOutputTemplateMetadata; | |
| /** | |
| * The schema for the config for the extractor in the `extractor.json` file. | |
| */ | |
| extractorConfigSchema?: TExtractorConfig; | |
| /** | |
| * The plugins to use for the extractor. | |
| */ | |
| pluginDependencies?: TPluginDependencies; | |
| /** | |
| * Extracts the metadata entries for the given files. | |
| * | |
| * @param files - The files to extract metadata from. | |
| * @param context - The context for the extractor. | |
| */ | |
| extractTemplateMetadataEntries( | |
| files: TemplateFileExtractorSourceFile<z.infer<TOutputTemplateMetadata>>[], | |
| context: TemplateExtractorContext<TPluginDependencies>, | |
| ): | |
| | Promise< | |
| TemplateFileExtractorMetadataEntry< | |
| z.infer<TGeneratorTemplateMetadata> | |
| >[] | |
| > | |
| | TemplateFileExtractorMetadataEntry<z.infer<TGeneratorTemplateMetadata>>[]; | |
| /** | |
| * Writes the files in the metadata entries to the generator's templates directory converting | |
| * any written templates to their templatized form. | |
| * | |
| * @param metadataEntries - The metadata entries to write. | |
| * @param context - The context for the extractor. | |
| * @param api - The API for the extractor. | |
| */ | |
| writeTemplateFiles( | |
| metadataEntries: TemplateFileExtractorMetadataEntry< | |
| z.infer<TGeneratorTemplateMetadata> | |
| >[], | |
| context: TemplateExtractorContext<TPluginDependencies>, | |
| api: TemplateExtractorApi, | |
| ): Promise<void>; | |
| /** | |
| * Writes the files in the metadata entries to the generator's generated directory creating | |
| * the necessary typed template files. | |
| * | |
| * @param generatorNames - The names of the generators to write. | |
| * @param context - The context for the extractor. | |
| * @param api - The API for the extractor. | |
| */ | |
| writeGeneratedFiles( | |
| generatorNames: string[], | |
| context: TemplateExtractorContext<TPluginDependencies>, | |
| api: TemplateExtractorApi, | |
| ): Promise<void> | void; | |
| } | |
| /** | |
| * The TemplateFileExtractor implements the core logic for extracting template | |
| * metadata from source files. | |
| * | |
| * It is responsible for: | |
| * - Extracting metadata from source files | |
| * - Writing metadata to the `extractor.json` file | |
| * - Writing template files to the package | |
| * | |
| * ## Lifecycle | |
| * 1. `extractTemplateMetadataEntries` - Called first to extract metadata from source files | |
| * 2. `writeTemplateFiles` - Called to write templatized versions of files to generator's templates directory | |
| * 3. Plugin hooks are executed (afterExtract) | |
| * 4. `writeGeneratedFiles` - Called to write typed template files to generator's generated directory | |
| * 5. Plugin hooks are executed (afterWrite in reverse dependency order) | |
| */ | |
| export interface TemplateFileExtractor< | |
| TGeneratorTemplateMetadata extends z.ZodSchema = typeof templateConfigSchema, | |
| TOutputTemplateMetadata extends | |
| z.ZodSchema = typeof templateFileMetadataBaseSchema, | |
| TExtractorConfig extends z.ZodSchema = z.ZodUnknown, | |
| TPluginDependencies extends | |
| TemplateExtractorPluginDependencies = TemplateExtractorPluginDependencies, | |
| > { | |
| /** | |
| * The name of the extractor. | |
| */ | |
| name: string; | |
| // ... rest of interface unchanged | |
| } |
🤖 Prompt for AI Agents
In packages/sync/src/templates/extractor-v2/runner/template-file-extractor.ts
around lines 57 to 140, the TemplateFileExtractor interface lacks documentation
on the lifecycle and order of method calls. Add a clear summary above the
interface explaining the extractor lifecycle: specify that
extractTemplateMetadataEntries is called first to extract metadata from source
files, then writeTemplateFiles uses that metadata to write template files in
their templatized form, and finally writeGeneratedFiles is called to write the
generated files based on the generator names. This will clarify the expected
sequence and relationships between these methods.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/core-generators/src/renderers/text/text-template-file-extractor.ts (1)
104-108:⚠️ Potential issueMissing await causes race condition with file container commit.
api.writeTemplateFileis not awaited, which can causefileContainer.commit()to execute before template writes complete, potentially resulting in partial or empty output.- api.writeTemplateFile( + await api.writeTemplateFile( file.generator, file.generatorTemplatePath, templateContents, );
🧹 Nitpick comments (1)
packages/core-generators/src/renderers/text/text-template-file-extractor.ts (1)
141-148: Simplify variable transformation logic.The destructuring and object reconstruction to remove the
valueproperty could be simplified using a more direct approach.- variables: ${JSON.stringify( - mapValues(config.variables ?? {}, (variable) => { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { value: _, ...variableWithoutValue } = - variable as TextTemplateFileVariableWithValue; - return variableWithoutValue; - }), - )}, + variables: ${JSON.stringify( + mapValues(config.variables ?? {}, ({ value, ...rest }) => rest), + )},
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core-generators/src/renderers/schemas/template-file-options.ts(1 hunks)packages/core-generators/src/renderers/templates/utils/package-path-specifier.ts(1 hunks)packages/core-generators/src/renderers/text/text-template-file-extractor.ts(1 hunks)packages/project-builder-server/src/sync/build-project.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/project-builder-server/src/sync/build-project.ts
- packages/core-generators/src/renderers/templates/utils/package-path-specifier.ts
- packages/core-generators/src/renderers/schemas/template-file-options.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test
- GitHub Check: Lint
- GitHub Check: Test E2E
🔇 Additional comments (2)
packages/core-generators/src/renderers/text/text-template-file-extractor.ts (2)
78-85: Good fix for the variable replacement order issue.The sorting by descending value length properly addresses the overlapping replacement concern from previous reviews, ensuring longer variable values are replaced first to prevent corruption.
96-100: Well-implemented validation for variable replacement.The string comparison check effectively validates that each variable value was found and replaced in the template content, providing clear error messages for debugging.
kingston
deleted the
kingston/eng-730-phase-1-foundation-create-configuration-schemas-and-basic
branch
Created parallel template extraction flow for text and raw generators
Summary by CodeRabbit
New Features
Improvements
.gitignorerules.Bug Fixes
Documentation
Tests
Chores
.knipignore pattern for generated files.