refactor: Introduce new TsImportMap system for import handling #487
Conversation
…iders - Also add ability to pass around imports via import maps
ENG-603 Introduce new system for passing around imports between providers
Currently we use importMaps but they are fragile and not typed so we should switch to a new system where we can pass in the |
|
WalkthroughThis pull request introduces extensive changes across multiple packages. Core modifications include replacing output provider types with read-only variants, updating asynchronous file generation tasks, and restructuring TypeScript generators. New files and functions related to import map transformations and topological sorting have been added, alongside comprehensive unit tests. Additionally, many import paths and exports have been updated to reflect the new module organization. Changes
Sequence Diagram(s)sequenceDiagram
participant Task as Generator Task
participant TS as TypescriptFile Provider
participant Builder as Task Builder
Task->>TS: writeTemplatedFile(builder, payload)
TS-->>Task: Promise resolves { destination }
sequenceDiagram
participant Caller as Caller
participant Transformer as transformTsImportsWithMap
participant Map as ImportMap
Caller->>Transformer: Provide import declarations & import map
alt Declaration without "%"
Transformer-->>Caller: Return unchanged declaration
else Declaration with "%"
Transformer->>Map: Lookup import key
alt Key not found or invalid import
Transformer-->>Caller: Throw error
else
Transformer-->>Caller: Return transformed declaration
end
end
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code Definitions (1)packages/fastify-generators/src/generators/core/config-service/index.ts (1)
🔇 Additional comments (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
packages/core-generators/src/renderers/typescript/import-maps/ts-import-map.ts (2)
55-61: Clarify error message about type-only imports.The error message when attempting to use a type-only import as a regular import could be more descriptive.
- throw new Error( - 'Type only imports cannot be marked as named imports', - ); + throw new Error( + `Cannot use type-only import '${name}' as a regular import. Use typeDeclaration() instead.`, + );
23-67:Details
❓ Verification inconclusive
Consider clarifying the source prefix convention.
The implementation looks solid, but I noticed that all import sources automatically get prefixed with
@/on line 39. This assumes a specific path alias configuration in the project.
🏁 Script executed:
#!/bin/bash # Check if the path alias is configured in the tsconfig grep -r "@/" --include="tsconfig*.json" .Length of output: 41
Clarify the use of the
@/source prefixThe implementation appears correct, but the code assumes that the
@/prefix is supported by a corresponding alias configuration—typically set up in a TypeScript configuration file (liketsconfig.json). However, our initial search did not reveal any such alias setting. Please verify that this alias is indeed configured in your project (or intended to be backed by another mechanism) and update the documentation or configuration as needed.packages/core-generators/src/renderers/typescript/import-maps/transform-ts-imports-with-map.ts (1)
19-56: Solid implementation of import transformation with good error handling.The function correctly handles various cases:
- Skips imports not starting with '%'
- Extracts import map keys
- Validates import map existence
- Checks for unsupported import types
- Properly transforms named imports based on whether they are type-only
Consider adding a comment explaining the behavior when
namedImportsis undefined - returning an empty array is correct but might not be immediately obvious to readers:return ( importDeclaration.namedImports?.map((namedImport) => { if (!(namedImport.name in importMap)) { throw new Error(`Import map entry not found for ${namedImport.name}`); } const entry = importMap[namedImport.name]; return importDeclaration.isTypeOnly ? entry.typeDeclaration() : entry.declaration(); - }) ?? [] + }) ?? [] // Return empty array if namedImports is undefined );packages/sync/src/runner/dependency-sort.unit.test.ts (1)
71-71: Revisiting provider naming.
createReadOnlyProviderType('output-provider')might be confusing. If genuinely read-only, consider revisiting the term “output” in the name for clarity.packages/utils/src/toposort/toposort.ts (1)
44-105: Depth-first search for topological sort.The DFS-based approach is solid. Consider documenting potential stack depth limits for extremely large graphs. Otherwise, the cycle detection and final ordering logic appear correct.
packages/fastify-generators/src/generators/core/logger-service/logger-service.generator.ts (1)
109-149: Asynchronous file templating logic.Switching to an async
buildstep to leveragetypescriptFile.writeTemplatedFileis an improvement. Consider error handling (try/catch) if file writes fail, possibly returning an informative log or fallback behavior.packages/sync/src/runner/dependency-map.ts (2)
29-34: NewEntryDependencyRecordinterface
The addition ofisReadOnlyandisOutputneatly distinguishes between read-only providers and output providers. Consider adding brief JSDoc comments explaining each field’s role (e.g., “isReadOnlyindicates …,” “isOutputindicates …”) for maintainability.
94-98: Duplicate provider detection
The logic to detect existing providers with the sameproviderIdand throw an error if found is crucial to avoid conflicting exports. The error message is very descriptive, though it might be beneficial to mention the exact export name in the error text for clarity.packages/core-generators/src/renderers/typescript/import-maps/types.ts (1)
3-14: IntroducingTsImportMapSchemaEntry
This interface effectively captures optional naming and a boolean flagisTypeOnly. The generic constraintIsTypeOnlyallows flexible usage. Consider adding doc comments to clarify howisTypeOnlyis employed in the broader system.packages/sync/src/providers/providers.ts (2)
27-29: Clarify documentation around read-only providers.These doc lines now correctly emphasize that only read-only providers can serve as outputs. However, consider explicitly linking the concept of “cannot modify state” with the requirement for outputs, so that future readers more readily understand the rationale.
133-133: Existing doc comment appears duplicated.Lines 27-29 have nearly identical statements regarding read-only providers being used as build outputs. Consider whether the duplication is necessary or if a single, centralized comment would be clearer.
packages/core-generators/src/renderers/typescript/renderers/file.ts (1)
28-31: Solid addition ofRenderTsCodeFileTemplateOptionsBaseandInferImportMapProvidersFromProviderTypeMap.The introduction of a base interface and a helper type for inferring provider types makes the code more modular and type-safe. Consider whether any of these fields should be mandatory in the base interface or remain optional, depending on usage patterns across the codebase.
Also applies to: 33-43
packages/core-generators/src/generators/node/typescript/index.ts (1)
286-345: Newfilegenerator task consolidates TypeScript file writing logic.
- Retrieving compiler options with defaults, generating path mappings, and constructing internal patterns ensures the environment is well prepared.
- The
typescriptFileprovider yields a clean async flow, callingrenderTsCodeFileTemplatewith the newimportMapProviders.- The code then writes to the builder and returns the destination.
Overall, this approach is easy to follow and maintain.
Consider documenting more detailed error handling for any I/O failures in the
writeTemplatedFilemethod if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
⛔ Files ignored due to path filters (3)
packages/fastify-generators/src/generators/core/config-service/generated/import-maps.tsis excluded by!**/generated/**packages/fastify-generators/src/generators/core/error-handler-service/generated/templates.tsis excluded by!**/generated/**packages/fastify-generators/src/generators/core/logger-service/generated/import-maps.tsis excluded by!**/generated/**
📒 Files selected for processing (49)
packages/core-generators/src/generators/node/typescript/index.ts(4 hunks)packages/core-generators/src/providers/project.ts(1 hunks)packages/core-generators/src/renderers/typescript/import-maps/index.ts(1 hunks)packages/core-generators/src/renderers/typescript/import-maps/transform-ts-imports-with-map.ts(1 hunks)packages/core-generators/src/renderers/typescript/import-maps/transform-ts-imports-with-map.unit.test.ts(1 hunks)packages/core-generators/src/renderers/typescript/import-maps/ts-import-map.ts(1 hunks)packages/core-generators/src/renderers/typescript/import-maps/types.ts(1 hunks)packages/core-generators/src/renderers/typescript/index.ts(1 hunks)packages/core-generators/src/renderers/typescript/renderers/file.ts(6 hunks)packages/core-generators/src/renderers/typescript/renderers/file.unit.test.ts(4 hunks)packages/core-generators/src/renderers/typescript/templates/creators.ts(2 hunks)packages/core-generators/src/renderers/typescript/templates/types.ts(3 hunks)packages/fastify-generators/src/generators/auth0/auth0-module/index.ts(1 hunks)packages/fastify-generators/src/generators/bull/bull-mq/index.ts(1 hunks)packages/fastify-generators/src/generators/core/_composers/fastify-composer.ts(1 hunks)packages/fastify-generators/src/generators/core/config-service/index.ts(4 hunks)packages/fastify-generators/src/generators/core/error-handler-service/index.ts(4 hunks)packages/fastify-generators/src/generators/core/error-handler-service/templates/plugins/error-handler.ts(1 hunks)packages/fastify-generators/src/generators/core/fastify-graceful-shutdown/index.ts(1 hunks)packages/fastify-generators/src/generators/core/fastify-server/index.ts(1 hunks)packages/fastify-generators/src/generators/core/fastify/index.ts(2 hunks)packages/fastify-generators/src/generators/core/index.ts(1 hunks)packages/fastify-generators/src/generators/core/logger-service/logger-service.generator.ts(4 hunks)packages/fastify-generators/src/generators/core/request-context/index.ts(1 hunks)packages/fastify-generators/src/generators/core/request-service-context/index.ts(2 hunks)packages/fastify-generators/src/generators/core/service-context/index.ts(2 hunks)packages/fastify-generators/src/generators/core/service-file/index.ts(2 hunks)packages/fastify-generators/src/generators/email/fastify-sendgrid/index.ts(1 hunks)packages/fastify-generators/src/generators/pothos/pothos/index.ts(3 hunks)packages/fastify-generators/src/generators/prisma/prisma-crud-service/index.ts(2 hunks)packages/fastify-generators/src/generators/prisma/prisma/index.ts(2 hunks)packages/fastify-generators/src/generators/stripe/fastify-stripe/index.ts(1 hunks)packages/fastify-generators/src/generators/yoga/yoga-plugin/index.ts(3 hunks)packages/react-generators/src/generators/admin/admin-crud-embedded-form/index.ts(2 hunks)packages/sync/src/providers/providers.ts(7 hunks)packages/sync/src/runner/dependency-map.ts(9 hunks)packages/sync/src/runner/dependency-map.unit.test.ts(30 hunks)packages/sync/src/runner/dependency-sort.ts(4 hunks)packages/sync/src/runner/dependency-sort.unit.test.ts(7 hunks)packages/sync/src/runner/generator-runner.ts(1 hunks)packages/sync/src/runner/generator-runner.unit.test.ts(5 hunks)packages/sync/src/runner/tests/dependency-entry.test-helper.ts(1 hunks)packages/sync/src/utils/create-config-provider-task-with-info.ts(2 hunks)packages/sync/src/utils/create-config-provider-task.ts(2 hunks)packages/tools/tsconfig.node.base.json(1 hunks)packages/utils/src/index.ts(1 hunks)packages/utils/src/toposort/index.ts(1 hunks)packages/utils/src/toposort/toposort.ts(1 hunks)packages/utils/src/toposort/toposort.unit.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (26)
packages/core-generators/src/renderers/typescript/import-maps/transform-ts-imports-with-map.unit.test.ts (3)
packages/core-generators/src/renderers/typescript/imports/types.ts (1)
TsImportDeclaration(22-43)packages/core-generators/src/renderers/typescript/import-maps/transform-ts-imports-with-map.ts (1)
transformTsImportsWithMap(19-56)packages/core-generators/src/renderers/typescript/import-maps/types.ts (1)
TsImportMap(30-30)
packages/core-generators/src/providers/project.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/react-generators/src/generators/admin/admin-crud-embedded-form/index.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/fastify-generators/src/generators/core/request-service-context/index.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/fastify-generators/src/generators/core/service-context/index.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/fastify-generators/src/generators/core/service-file/index.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/fastify-generators/src/generators/core/fastify/index.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/sync/src/utils/create-config-provider-task.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/fastify-generators/src/generators/prisma/prisma/index.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/fastify-generators/src/generators/prisma/prisma-crud-service/index.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/fastify-generators/src/generators/yoga/yoga-plugin/index.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/core-generators/src/renderers/typescript/renderers/file.unit.test.ts (2)
packages/core-generators/src/renderers/typescript/renderers/file.ts (1)
renderTsCodeFileTemplate(117-177)packages/core-generators/src/renderers/typescript/ts-code-utils.ts (1)
template(161-182)
packages/sync/src/runner/tests/dependency-entry.test-helper.ts (1)
packages/sync/src/runner/dependency-map.ts (1)
EntryDependencyRecord(29-34)
packages/sync/src/runner/generator-runner.unit.test.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/sync/src/runner/dependency-sort.ts (1)
packages/utils/src/toposort/toposort.ts (1)
toposort(51-104)
packages/sync/src/runner/dependency-map.unit.test.ts (2)
packages/sync/src/providers/providers.ts (2)
createProviderType(144-203)createReadOnlyProviderType(212-217)packages/sync/src/runner/tests/dependency-entry.test-helper.ts (3)
createReadOnlyDependencyEntry(22-30)createDependencyEntry(3-20)createOutputDependencyEntry(32-45)
packages/sync/src/runner/dependency-map.ts (1)
packages/sync/src/providers/providers.ts (1)
ProviderExport(101-122)
packages/fastify-generators/src/generators/core/logger-service/logger-service.generator.ts (4)
packages/core-generators/src/generators/node/typescript/index.ts (1)
typescriptFileProvider(121-122)packages/fastify-generators/src/generators/core/logger-service/generated/import-maps.ts (2)
loggerServiceImportsProvider(17-20)createLoggerServiceImportMap(26-32)packages/fastify-generators/src/generators/core/logger-service/generated/templates.ts (1)
loggerFileTemplate(4-12)packages/core-generators/src/renderers/typescript/ts-code-utils.ts (1)
TsCodeUtils(38-183)
packages/fastify-generators/src/generators/pothos/pothos/index.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/core-generators/src/renderers/typescript/templates/types.ts (1)
packages/sync/src/providers/providers.ts (1)
ProviderType(17-40)
packages/fastify-generators/src/generators/core/error-handler-service/index.ts (4)
packages/fastify-generators/src/generators/core/config-service/generated/import-maps.ts (1)
configServiceImportsProvider(17-20)packages/fastify-generators/src/generators/core/logger-service/logger-service.generator.ts (1)
loggerServiceProvider(50-53)packages/core-generators/src/generators/node/typescript/index.ts (1)
typescriptFileProvider(121-122)packages/fastify-generators/src/generators/core/error-handler-service/generated/templates.ts (1)
errorHandlerPluginFileTemplate(6-18)
packages/sync/src/utils/create-config-provider-task-with-info.ts (1)
packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/sync/src/runner/dependency-sort.unit.test.ts (2)
packages/sync/src/runner/tests/dependency-entry.test-helper.ts (3)
createDependencyEntry(3-20)createOutputDependencyEntry(32-45)createReadOnlyDependencyEntry(22-30)packages/sync/src/providers/providers.ts (1)
createReadOnlyProviderType(212-217)
packages/core-generators/src/renderers/typescript/import-maps/transform-ts-imports-with-map.ts (2)
packages/core-generators/src/renderers/typescript/imports/types.ts (1)
TsImportDeclaration(22-43)packages/core-generators/src/renderers/typescript/import-maps/types.ts (1)
TsImportMap(30-30)
packages/core-generators/src/renderers/typescript/import-maps/types.ts (1)
packages/core-generators/src/renderers/typescript/imports/types.ts (1)
TsImportDeclaration(22-43)
packages/core-generators/src/renderers/typescript/renderers/file.ts (4)
packages/core-generators/src/renderers/typescript/imports/types.ts (1)
TsImportDeclaration(22-43)packages/core-generators/src/renderers/typescript/import-maps/types.ts (2)
TsImportMap(30-30)TsImportMapProvider(36-38)packages/core-generators/src/renderers/typescript/imports/ts-morph-operations.ts (1)
getTsMorphImportDeclarationsFromSourceFile(19-25)packages/core-generators/src/renderers/typescript/templates/types.ts (1)
TsCodeFileTemplate(19-38)
🔇 Additional comments (126)
packages/react-generators/src/generators/admin/admin-crud-embedded-form/index.ts (2)
12-14: Provider type import change aligns with output provider refactoring.The import of
createReadOnlyProviderTypefrom@halfdomelabs/syncaligns with the PR objective to refactor output providers to read-only providers.
132-135: Changed output provider to read-only provider.This change from
createOutputProviderTypetocreateReadOnlyProviderTypeis consistent with the PR objective of switching to read-only providers exclusively instead of output providers.The provider's type signature remains unchanged, preserving its functionality while making it immutable, which is good for preventing unexpected modifications during the generation process.
packages/utils/src/toposort/index.ts (1)
1-2: Clean barrel file for toposort module.This is a simple and effective re-export pattern (barrel file) for the toposort module, making it easier to import from other parts of the codebase.
packages/utils/src/index.ts (1)
6-7: Correctly exports the toposort module.This export makes the toposort functionality available to consumers of the utils package, following the same pattern used for other modules in this file.
packages/tools/tsconfig.node.base.json (1)
5-7: Improved file watching configuration for TypeScript.The addition of the
watchOptionsconfiguration with"watchFile": "usefseventsonparentdirectory"can improve TypeScript's file watching performance by using file system events on parent directories rather than polling individual files.This is particularly useful in large projects where many files need to be watched during development, as it can reduce CPU usage and be more responsive to changes.
packages/fastify-generators/src/generators/core/fastify-graceful-shutdown/index.ts (1)
11-11: Import path refactored to be more specific.The import path for
loggerServiceProviderhas been updated to point directly to the generator file instead of the index file. This change is part of the new TsImportMap system being introduced, making import paths more explicit and maintainable.packages/core-generators/src/renderers/typescript/index.ts (1)
2-2: New export added for import maps system.This addition exports all the definitions from the import-maps module, which is a key component of the new TsImportMap system mentioned in the PR objectives. This change enables the use of import maps throughout the codebase.
packages/fastify-generators/src/generators/core/request-context/index.ts (1)
24-24: Import path refactored to be more specific.The import path for
loggerServiceSetupProviderhas been updated to point directly to the generator file instead of the index file. This change is consistent with the new TsImportMap system being introduced in this PR.packages/fastify-generators/src/generators/core/index.ts (1)
14-14: Export path refactored to be more specific.The export path for the logger service has been updated to point directly to the generator file instead of the index file. This change is consistent with the pattern of making import/export paths more explicit throughout the codebase as part of the new TsImportMap system.
packages/fastify-generators/src/generators/auth0/auth0-module/index.ts (1)
26-26: Import path update looks good!This change from importing from
index.jsto the more specificlogger-service.generator.jsaligns with the broader restructuring of logger service imports across the codebase. This approach improves code organization by making import paths more explicit and consistent.packages/core-generators/src/renderers/typescript/import-maps/index.ts (1)
1-3: Well-structured barrel file for the new import map system!This clean organization exports functionality from three distinct modules, separating concerns between transformation logic, import map functionality, and type definitions. The use of
export type *for types is particularly good practice.packages/core-generators/src/providers/project.ts (1)
1-1: Good transition to readOnly provider type!Changing from
createOutputProviderTypetocreateReadOnlyProviderTypealigns with the PR objective of refactoring output providers to use readOnly providers exclusively. This is a good design decision for the project provider, as it prevents unintended modifications and enforces a clearer consumption pattern.Also applies to: 8-8
packages/fastify-generators/src/generators/core/_composers/fastify-composer.ts (1)
16-16: Consistent import path update!This change from
../logger-service/index.jsto the more specific../logger-service/logger-service.generator.jsmatches the pattern seen across the codebase. It's part of a consistent effort to make import paths more explicit and better organized.packages/fastify-generators/src/generators/bull/bull-mq/index.ts (1)
20-20: Import path update looks good.The import path for
loggerServiceProviderhas been updated to use a more specific path, pointing directly to the generator file. This change is consistent with the refactoring approach described in the PR objectives to introduce the new TsImportMap system.packages/fastify-generators/src/generators/email/fastify-sendgrid/index.ts (1)
16-16: Import path update looks good.The import path for
loggerServiceProviderhas been updated to use a more specific module path, pointing directly to the generator file. This change is consistent with the pattern seen across other files and aligns with the TsImportMap system introduction mentioned in the PR objectives.packages/fastify-generators/src/generators/stripe/fastify-stripe/index.ts (1)
19-19: Import path update looks good.The import path for
loggerServiceProviderhas been updated to use a more specific path, pointing directly to the generator file. This change follows the same pattern as in other files and supports the new TsImportMap system implementation.packages/core-generators/src/renderers/typescript/import-maps/transform-ts-imports-with-map.unit.test.ts (1)
1-158: Comprehensive unit tests for the new import transformation system.This test file thoroughly covers the functionality of the
transformTsImportsWithMapfunction, which is a key component of the new TsImportMap system. The tests handle various cases including regular imports, error conditions, and different import types.The tests are well-structured and cover:
- Standard imports without transformation
- Error handling for missing import maps and unsupported import types
- Proper transformation of named and type-only imports
- Handling of multiple imports with different target modules
This implementation will help ensure the robustness of the new import handling system.
packages/fastify-generators/src/generators/core/error-handler-service/templates/plugins/error-handler.ts (1)
6-6: Import path updated to use configServiceThe import path has been updated from
%configto%configService, which aligns with the PR objective of introducing a new TsImportMap system for import handling. This change maintains backward compatibility since the code still references theconfigvariable in the same way.packages/fastify-generators/src/generators/core/service-file/index.ts (2)
14-14: Added import for createReadOnlyProviderTypeThis import is necessary for the change from output providers to read-only providers as mentioned in the PR objectives.
52-52: Switched to read-only provider typeThe provider type has been changed from output to read-only, which improves type safety by preventing modification of the provider's state after initialization. This change aligns with the PR objective of utilizing readOnly providers exclusively instead of output providers.
packages/fastify-generators/src/generators/core/request-service-context/index.ts (2)
18-18: Added import for createReadOnlyProviderTypeThis import is necessary for the change from output providers to read-only providers as mentioned in the PR objectives.
58-58: Switched to read-only provider typeThe provider type has been changed from output to read-only, which improves type safety by preventing modification of the provider's state after initialization. This change aligns with the PR objective of utilizing readOnly providers exclusively instead of output providers.
packages/fastify-generators/src/generators/core/fastify-server/index.ts (1)
25-25: Updated import path for loggerServiceProviderThe import path has been changed from
../logger-service/index.jsto../logger-service/logger-service.generator.js, which is more specific and better reflects the purpose of the imported module. This is consistent with the PR objective of refactoring import handling.packages/sync/src/runner/generator-runner.ts (2)
95-100: Migration fromisOutputtoisReadOnlyThis code correctly updates the dependency property destructuring to use
isReadOnlyinstead of the previousisOutput. This is part of the new TsImportMap system being introduced in this PR.
105-108: Updated dependency validation for read-only providersThe condition has been updated to check
isReadOnlyinstead of the previousisOutputflag, with a corresponding error message update. This correctly aligns with the architectural shift from output providers to read-only providers.packages/fastify-generators/src/generators/core/fastify/index.ts (2)
16-16: Added import for createReadOnlyProviderTypeThe import statement has been updated to include
createReadOnlyProviderTypefor the new provider implementation.
63-64: Updated fastifyOutputProvider to use read-only typeThe provider type has been changed from
createOutputProviderTypetocreateReadOnlyProviderType, which aligns with the architecture change from output providers to read-only providers.This change maintains the same interface while changing the provider characteristics to be read-only, ensuring compatibility with the new system.
packages/fastify-generators/src/generators/core/service-context/index.ts (2)
17-17: Added import for createReadOnlyProviderTypeThe import statement has been updated to include
createReadOnlyProviderTypefor the new provider implementation.
49-50: Updated serviceContextProvider to use read-only typeThe provider type has been changed from
createOutputProviderTypetocreateReadOnlyProviderType, aligning with the architecture changes in this PR. The interface remains unchanged, ensuring backward compatibility while implementing the new provider type system.packages/core-generators/src/renderers/typescript/renderers/file.unit.test.ts (4)
21-23: Updated test function call with importMapProviders parameterThis test case has been updated to include the new
importMapProvidersparameter required by the modifiedrenderTsCodeFileTemplatefunction. The empty object initialization ensures backward compatibility with existing tests.
47-49: Added importMapProviders parameter to test callThe test case for merging and sorting imports has been properly updated to include the
importMapProvidersparameter, ensuring compatibility with the updated function signature.
78-81: Added importMapProviders parameter to module resolution testThe test for module resolution has been properly updated to include the
importMapProvidersparameter, maintaining consistency across all test cases.
120-122: Added importMapProviders parameter to hoisted fragments testThe test for hoisted fragments has been properly updated to include the
importMapProvidersparameter, ensuring all test cases consistently use the new function signature.packages/sync/src/utils/create-config-provider-task-with-info.ts (2)
13-14: Import replacement aligns with provider refactoring.Adding
createReadOnlyProviderTypeto replace the removedcreateOutputProviderTypeis consistent with the PR objective of refactoring output providers to read-only providers.
111-113: Provider type change correctly implements read-only behavior.Changing from
createOutputProviderTypetocreateReadOnlyProviderTypealigns perfectly with the provider's intended functionality as described in the comment on line 68: "exposing the resolved configuration values (read-only)". This makes the provider's read-only nature explicit in its definition rather than just in documentation.packages/utils/src/toposort/toposort.unit.test.ts (6)
1-8: Well-structured imports and dependencies.The imports are properly organized, with test framework utilities first, followed by the module under test.
9-39: Excellent helper function implementation.The
expectOrderhelper function is well-designed to verify that the sorted output respects all dependencies. It meticulously checks that for each edge, the source node appears before the target node in the sorted output, with clear error messages for diagnostics.
40-109: Comprehensive test cases for happy paths.These tests thoroughly cover the expected behaviors of the
toposortfunction, including:
- Empty graphs
- Single node graphs
- Linear chains (with both string and number nodes)
- Multiple paths with branching dependencies
- Disconnected components
- Nodes with no edges
The tests are well-organized, with clear expectations and validations using both exact equality and the custom
expectOrderhelper.
113-180: Thorough error handling tests for cyclical dependencies.The test cases appropriately verify that the function throws the correct error type (
ToposortCyclicalDependencyError) with the expected cycle path for various cycle scenarios:
- Simple cycles between two nodes
- Longer cycles involving multiple nodes
- Self-loops
The tests check both the error type and the detailed error information, ensuring complete validation of error behavior.
182-210: Robust validation of unknown node error handling.These tests properly verify that the function throws
ToposortUnknownNodeErrorwhen nodes referenced in edges don't exist in the input list. Testing both unknown source and target nodes ensures complete coverage of this error condition.
212-238: Excellent edge case handling.The final tests confirm the function's ability to handle:
- Object nodes using reference equality
- Duplicate edges gracefully
These edge cases are important for robust implementation and show attention to detail in the testing strategy.
packages/sync/src/utils/create-config-provider-task.ts (2)
17-19: Import replacement aligns with provider refactoring.Adding
createReadOnlyProviderTypeto replace the removedcreateOutputProviderTypeis consistent with the PR objective of refactoring output providers to read-only providers.
100-102: Provider type change correctly implements read-only behavior.Changing from
createOutputProviderTypetocreateReadOnlyProviderTypealigns with the provider's intended functionality as described in the comment on line 60: "exposing the resolved configuration values (read-only)". This change is consistent with the similar modification increate-config-provider-task-with-info.ts, showing a systematic approach to this refactoring.packages/fastify-generators/src/generators/prisma/prisma/index.ts (2)
18-19: Import replacement aligns with provider refactoring.Adding
createReadOnlyProviderTypeto replace the removedcreateOutputProviderTypeis consistent with the PR objective of refactoring output providers to read-only providers.
66-67: Appropriate provider type for read-only operations.Changing
prismaOutputProviderto usecreateReadOnlyProviderTypeis appropriate since thePrismaOutputProviderinterface only defines getter methods that retrieve information without modifying state. This aligns perfectly with making it a read-only provider and follows the consistent pattern of provider refactoring across the codebase.packages/fastify-generators/src/generators/yoga/yoga-plugin/index.ts (3)
21-21: Good refactoring: Using read-only providers instead of output providersThis change aligns with the PR objective of changing output providers to read-only providers, which generally improves immutability and reduces potential side effects.
35-35: Path updated for loggerServiceProvider importThe import path for
loggerServiceProviderhas been updated to point to a more specific location, which helps with better code organization and clarity.
78-80: Provider type changed to read-onlyThe
yogaPluginSetupProviderhas been updated to usecreateReadOnlyProviderTypeinstead ofcreateOutputProviderType, which enforces immutability of the provider data.packages/core-generators/src/renderers/typescript/templates/creators.ts (1)
1-1: Enhanced template function with import providers supportThe
tsCodeFileTemplatefunction has been enhanced with a new generic type parameterTImportProvidersthat allows for better type-checking and integration with the new TsImportMap system. This change improves type safety while maintaining backward compatibility by providing a default empty record.Also applies to: 13-19
packages/fastify-generators/src/generators/core/config-service/index.ts (4)
29-32: Added new import map providersNew imports for
configServiceImportsProviderandcreateConfigServiceImportMaphave been added, which aligns with the PR objective of introducing the new TsImportMap system for better import handling.
84-87: Updated exports to include configServiceImportsThe
exportsobject has been extended to includeconfigServiceImports, making the import map provider available at the project scope, which is necessary for the new import handling system.
124-127: Added configServiceImports providerThe implementation properly configures the import map for the config service, specifying the service path. This creates a centralized way to manage imports for the config service throughout the codebase.
210-210: Re-exported configServiceImportsProviderThe module re-exports the
configServiceImportsProviderto make it available for use in other modules. This is good practice for encapsulation while providing necessary functionality to external modules.packages/fastify-generators/src/generators/prisma/prisma-crud-service/index.ts (1)
7-7: Converted output provider to read-only providerThe
prismaCrudServiceProvideris now created usingcreateReadOnlyProviderTypeinstead ofcreateOutputProviderType. This change:
- Aligns with the PR's objective of refactoring output providers to use read-only providers
- Enforces immutability which reduces potential side effects
- Makes the code more robust and predictable
This is part of a consistent pattern across multiple files in this PR.
Also applies to: 32-32
packages/sync/src/runner/tests/dependency-entry.test-helper.ts (1)
1-46: Well-designed helper functions for test dependency creationThis new file introduces clean, well-structured helper functions for creating test dependency entries. The code follows good practices with proper type annotations, clear function names, and effective use of the nullish coalescing operator for default values.
The functions provide a consistent interface for creating different types of dependency entries:
createDependencyEntryas the base functioncreateReadOnlyDependencyEntryfor read-only entriescreateOutputDependencyEntryfor output entries (which are inherently read-only)This approach will improve test readability and maintainability across the codebase.
packages/sync/src/runner/generator-runner.unit.test.ts (5)
17-19: Import update aligned with provider type refactoringThe import now correctly includes
createReadOnlyProviderTypeinstead of the removedcreateOutputProviderType, aligning with the PR's objective of refactoring output providers to use read-only providers.
221-224: Output provider replaced with read-only providerThe change from
createOutputProviderTypetocreateReadOnlyProviderTypecorrectly implements the new provider model. This test now validates that read-only providers work properly in the context previously used for output providers.
340-347: Updated provider types in phased task execution testBoth
mainOutputProviderTypeandphase1OutputProviderTypeare now correctly created usingcreateReadOnlyProviderTypeinstead ofcreateOutputProviderType, maintaining the test's functionality while conforming to the new provider model.
461-462: Updated error message to reflect new provider modelThe error message now correctly references "read-only" status rather than "output" status, accurately reflecting the new requirement that dependencies from previous phases must be read-only.
620-623: Updated provider type in dynamic tasks testThe dynamic task test now correctly uses
createReadOnlyProviderTypeinstead ofcreateOutputProviderType, aligning with the updated provider model.packages/core-generators/src/renderers/typescript/templates/types.ts (2)
1-3: Added imports for new TypeScript import handling systemThe new imports for
ProviderTypeandTsImportMapProvidersupport the TsImportMap system being introduced in this PR, enabling stronger typing for import maps.
19-38: Enhanced TsCodeFileTemplate to support import map providersThe interface has been expanded with:
- A new generic type parameter
TImportMapProviderswith a sensible default- A new optional
importMapProvidersproperty with clear documentationThis enhancement allows for specifying import map providers while maintaining backward compatibility, supporting the new import handling system without breaking existing code.
packages/sync/src/runner/dependency-sort.ts (3)
1-1: Updated toposort import sourceThe import has been changed from default to named import from '@halfdomelabs/utils', reflecting the reorganization of utility functions.
84-84: Updated toposort function callThe call to
toposorthas been updated to match its new signature as a named import, keeping the functionality consistent.
53-55:Details
✅ Verification successful
Simplified property access for dependency metadata
The code now directly accesses
isOutputandisReadOnlyproperties from the dependent object instead of using optional chaining and nullish coalescing. Ensure that these properties are always defined on thedependentobject to avoid potential undefined errors.
🏁 Script executed:
#!/bin/bash # Check if EntryDependencyRecord always defines isOutput and isReadOnly properties echo "Checking EntryDependencyRecord interface definition..." rg -A 10 "interface EntryDependencyRecord" packages/sync/src/Length of output: 894
Verified Dependency Property Access is Safe
The change to directly access
dependent.isOutputanddependent.isReadOnlyis valid because theEntryDependencyRecordinterface (defined inpackages/sync/src/runner/dependency-map.ts) guarantees that both properties exist as booleans. No further issues were found.
- Confirmed that the
EntryDependencyRecordinterface definesisOutputandisReadOnlyas boolean.- No additional refactor is necessary.
packages/core-generators/src/renderers/typescript/import-maps/ts-import-map.ts (3)
9-13: Nice type-safe schema creation function.This function provides a simple but effective way to enforce type safety on import schemas without adding runtime overhead.
15-21: LGTM: Well-structured input type definition.The type allows for both simple string paths and objects with source properties, providing flexibility in how imports are defined.
69-74: LGTM: Provider creation function is simple and effective.This function composes nicely with
createTsImportMapto create a provider for the import map schema.packages/sync/src/runner/dependency-map.unit.test.ts (4)
17-17: Good refactoring to support readOnly providers.The transition from output providers to readOnly providers is well-handled with proper imports and type creation.
Also applies to: 23-27, 35-35
112-116: Improved test readability with helper functions.Using the helper functions like
createDependencyEntryandcreateReadOnlyDependencyEntryimproves code consistency and readability compared to inline objects.Also applies to: 194-197, 249-252, 255-258, 461-464, 467-470
538-539: Successfully migrated providers to readOnly pattern.All output providers have been effectively migrated to readOnly providers, maintaining the same functionality while aligning with the new pattern.
Also applies to: 549-549, 562-565, 579-580, 586-587, 600-603, 639-640, 647-648, 672-675, 750-751, 768-769, 782-783, 807-810, 816-819, 826-829, 864-867, 926-929, 935-938, 975-976, 992-995
608-608: Terminology updated from "non-output provider" to "mutable provider".The error message and test description have been updated to reflect the new terminology, which is more accurate and descriptive.
Also applies to: 625-625
packages/fastify-generators/src/generators/pothos/pothos/index.ts (1)
25-25: Successfully migrated Pothos providers to readOnly pattern.The Pothos generator has been properly updated to use
createReadOnlyProviderTypeinstead of output providers, maintaining the same structure and type definitions.Also applies to: 54-58, 68-70
packages/core-generators/src/renderers/typescript/import-maps/transform-ts-imports-with-map.ts (1)
4-18: Well-documented function with comprehensive JSDoc.The JSDoc comment clearly explains the function's purpose, behavior, and parameters, making it easy for other developers to understand and use.
packages/sync/src/runner/dependency-sort.unit.test.ts (8)
5-5: Adopted read-only provider.Using
createReadOnlyProviderTypefor the output provider aligns with the new read-only approach. Ensure the name "output-provider" is still appropriate, given it's now read-only.
8-12: Centralized dependency creation.Switching to
createDependencyEntry,createOutputDependencyEntry, andcreateReadOnlyDependencyEntrystandardizes how dependencies are defined, reducing code duplication and improving clarity.
29-34: Use ofcreateDependencyEntry.These lines correctly replace inline dependency objects with function calls, making the test setup more consistent.
37-44: Consistent dependency usage.Again, good consistency in applying
createDependencyEntryfor building the dependency graph.
95-99: Correct usage ofcreateOutputDependencyEntry.This mapping ensures consumer tasks depend on the producer’s output. The approach is clear and straightforward.
140-149: Multiple dependency definitions.Combining
createOutputDependencyEntryandcreateDependencyEntryin one dependency map is well-handled. No issues here.
181-185: Read-only dependencies.Using
createReadOnlyDependencyEntryfor the consumer matches the test’s intention. No issues found.
232-246: Complex dependency chain.These entries correctly showcase mixed provider types (
outputDependencyEntry,createDependencyEntry,createReadOnlyDependencyEntry). The test thoroughly covers multi-tier dependencies.packages/utils/src/toposort/toposort.ts (3)
1-10: Custom cyclical dependency error class.Excellent job encapsulating cycle path details in
ToposortCyclicalDependencyError. This makes debugging cycles more straightforward.
12-19: Custom unknown node error class.
ToposortUnknownNodeErrorneatly isolates missing-node issues. This separation from cyclical errors improves error specificity.
21-42: Outgoing edges creation with validation.The
makeOutgoingEdgesfunction is well-structured, ensuring non-existent nodes are caught early. Good robust error checks.packages/fastify-generators/src/generators/core/logger-service/logger-service.generator.ts (5)
13-13: AddedtypescriptFileProviderto dependencies.This aligns the generator tasks with the newly introduced read-only TypeScript file handling system. No concerns here.
25-28: Importing logger service import map artifacts.Bringing in
createLoggerServiceImportMapandloggerServiceImportsProviderclarifies how logger imports are handled.
67-67: Provider dependency update.Ensuring
typescriptFileProvideris part of the dependencies is crucial for file template operations. Looks good.
72-74: New output for logger service imports.Exporting
loggerServiceImportshelps decouple import management from the main logger provider logic.
158-159: Re-exporting the imports provider.Re-exporting
loggerServiceImportsProvideraligns with the overall pattern of making key providers available at the generator's entry points.packages/sync/src/runner/dependency-map.ts (11)
10-10: Use ofProviderExporttype
The introduction ofProviderExportat line 10 looks consistent with the new logic handling provider exports. No immediate concerns here.
20-20: Updatedprovidersmap structure
Changingprovidersto store an object{ taskId: string; isOutput: boolean }rather than a simple string reference clarifies usage context. This can help in subsequent logic (e.g., checking for duplicates, determining output status).
36-39: Nested record type forEntryDependencyMap
Using a nestedRecord<string, Record<string, EntryDependencyRecord | null | undefined>>is a succinct approach for organizing dependencies. Verify that passingnull | undefinedis intentional and consistently handled throughout the codebase.
59-59: Filtering for non-read-only outputs
This filter ensures that any task outputs that are not read-only are flagged as invalid. This enforces the requirement that outputs must strictly be read-only, preventing configuration mistakes.
63-63: Error message clarity
The error text clearly identifies the invalid provider outputs. The usage ofinvalidTaskOutputs.map((output) => output.name)is helpful to inform the user exactly which outputs are invalid.
68-71:addTaskExporthelper function
The helper cleanly encapsulates adding exports while respectingisOutput. Good job keeping the parameters minimal and explicit.
103-106: StoringisOutputin the provider map
This approach effectively captures output status for downstream decisions. The chosen naming is consistent with the property name from the function parameter.
153-154: DestructuringisReadOnly& dependency options
Destructuring is straightforward. Ensure the upstream code always populates these fields (likenormalizedDep.options) to avoid possible undefined property scenarios.
170-172: Resolving providers fromgeneratorIdToScopesMap
The condition checks if a matching provider is found only within the relevant parent scope. This helps keep dependencies from leaking outside their intended scope.
174-174: Error on unresolvable dependency
Throwing here avoids incorrectly setting up the task with incomplete provider references. Looks correct.
195-196: CombiningisOutputwithisReadOnly
Bundling both states into the final dependency record is logical. Confirm that the rest of the pipeline correctly handles scenarios whereisOutputandisReadOnlymight conflict.packages/core-generators/src/renderers/typescript/import-maps/types.ts (7)
1-1: ImportingTsImportDeclaration
The usage of a separate import forTsImportDeclarationhelps keep the code modular. No issues found here.
16-16:TsImportMapSchema
A simple record type that ties string keys toTsImportMapSchemaEntry. Straightforward and easy to reason about.
18-28:TsImportMapEntryinterface
The approach to differentiatedeclaration(...)andtypeDeclaration(...)is neat. The generic constraintIsTypeOnly extends boolean | undefinedis a clever way to produce a compile-time error (never) ifisTypeOnlyis alwaystrue. This design is advanced but very useful for type integrity.
30-30: Definition ofTsImportMap
This record ofTsImportMapEntryprovides a direct mapping from string keys to their imported declarations. No issues found.
32-34:InferTsImportMapFromSchema
Inferring a map entry fromTsImportMapSchemaensures consistency throughout the code. This strongly enforces the typed relationship between schema definitions and actual import map entries.
36-38:TsImportMapProviderinterface
Encapsulating the entire map underimportMapis a clean pattern for providers. This design is easy to integrate with external code.
40-41: Combining schema with provider
TsImportMapProviderFromSchemaneatly composes the inferred map with the provider to produce a typed provider object. This is a well-structured approach.packages/fastify-generators/src/generators/core/error-handler-service/index.ts (7)
11-11: Refactoring to usetypescriptFileProvider
Bringing intypescriptFileProviderhelps unify file operations under the same interface for generating code. This approach is consistent across the codebase.
22-22: Replacing directconfigServiceProviderwithconfigServiceImportsProvider
Switching to a read-only provider for config imports indicates a shift away from mutable config usage. This can reduce side effects and ensure immutability of shared resources.
24-25: Reorganized imports for logger & error handler template
UsingloggerServiceProviderfromlogger-service.generator.jsand referencingerrorHandlerPluginFileTemplatefrom./generated/templates.jscentralizes template logic. This makes future maintenance easier.
70-70: AddingconfigServiceImportsas a task dependency
Explicitly listingconfigServiceImportsindicates the generator relies on specialized config import functionality. This is a more modular approach than pulling in the entire config service.
74-74: IntroducingtypescriptFilefor file generation tasks
Depending ontypescriptFileProviderclarifies that this step involves writing TypeScript files, aligning with the read-only provider pattern.
80-85: Expanded parameters inrun
The new signature fetchesconfigServiceImportsandtypescriptFile. This is consistent with the shift to read-only providers. It also declutters the method by removing less relevant provider calls.
124-132: UsingtypescriptFile.writeTemplatedFilewithimportMapProviders
This is a clean integration of the new approach—usingerrorHandlerPluginFileTemplatewhile injectingconfigServiceimports. It further showcases how read-only providers unify the injection of external resources without side effects.packages/sync/src/providers/providers.ts (4)
68-68: Confirm backward compatibility forProviderDependency.isReadOnly.This field has changed from being optional to mandatory. Please verify that all existing references to
ProviderDependencyinstances are updated so they explicitly configure or handleisReadOnly.
105-105: AlignProviderExport.isReadOnlywith final spec.Similar to the above dependency property, this export property is now always
trueorfalse. Ensure consumers ofProviderExportexpect it to be strictly defined.
157-163: Check defaults forisReadOnlylogic.Here,
isReadOnlyis defaulted tofalsewhen the user doesn’t specify otherwise. Confirm that this default strategy won’t lead to confusion for tasks where output usage is not intended, or that callers definitely override it if they need read-only behavior.Also applies to: 193-193
212-214: NewcreateReadOnlyProviderTypefunction looks solid.This utility ensures a provider is always read-only. There are no obvious issues, but you might consider whether any further checks are needed to ensure an already read-only provider isn’t re-wrapped or forced into another read-only instance redundantly.
Also applies to: 216-216
packages/core-generators/src/renderers/typescript/renderers/file.ts (4)
1-1: New imports provide clarity around the use of TsImportMap.Importing
TsImportMap,TsImportMapProvider, andtransformTsImportsWithMapintegrates seamlessly with the rest of the file. No apparent issues found here.Also applies to: 8-8, 18-18
49-50:mergeImportsAndHoistedFragmentsnow supports import maps.By passing
importMaps: Map<string, TsImportMap>and transforming the existing imports, the solution appears clean. Keep an eye on performance if the number of import maps grows large, though for most scenarios this should be efficient.Also applies to: 58-62
117-126: ExtendedrenderTsCodeFileTemplatewithTImportMapProviders.Allowing a generic for the import-map providers improves flexibility and type-safety. Confirm that all call sites now pass
importMapProviderswhen needed or else handle them gracefully if defaulted.
140-140: Conditional return and import map providers map construction.
- The early return (line 140) avoids overhead when no imports, fragments, or providers are relevant, which is a helpful optimization.
- The creation of
importMapProvidersMapensures each provider has the requiredimportMapproperty, throwing if not.- Passing
importMapProvidersMaptomergeImportsAndHoistedFragmentsis well-structured.Everything looks consistent in error handling and usage.
Also applies to: 154-164, 172-173
packages/core-generators/src/generators/node/typescript/index.ts (3)
3-5: Imports reorganize references toGeneratorTaskOutputBuilder,ProviderType, andInferImportMapProvidersFromProviderTypeMap.This reorganization is coherent and aligns with changes in the rest of the codebase. No concerns here.
Also applies to: 21-21
93-105:WriteTemplatedFilePayloadinterface additions add clarity.Adding
idandimportMapProvidersclarifies how generated templates are identified and how map providers are injected. Make sure the rest of the pipeline or any preexisting usage ofWriteTemplatedFilePayloadis updated accordingly.
109-118: Signature changes forwriteTemplatedFileeffectively support import maps.Requiring
TImportMapProvidersin the payload ensures the function can handle these providers robustly. This is a welcome enhancement for typed usage.
kingston
deleted the
kingston/eng-603-introduce-new-system-for-passing-around-imports-between
branch
Summary by CodeRabbit
New Features
Refactor
Chores & Tests