-
-
Notifications
You must be signed in to change notification settings - Fork 104
perf: improve TS code generation performance #2082
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThe changes refactor code generation modules by replacing the use of Changes
Sequence Diagram(s)sequenceDiagram
participant PG as PolicyGenerator
participant FW as FastWriter (CodeWriter)
participant EF as ExtraFunctions
participant SF as SourceFile
PG->>FW: Initialize code writer instance
PG->>FW: Write policy definitions (writePolicy, etc.)
PG->>EF: Collect extra function declarations
PG->>SF: Append extra functions to policy content
SF-->>PG: Return final policy file
sequenceDiagram
participant MM as ModelMetaGenerator
participant FW as FastWriter (CodeWriter)
participant EF as ExtraFunctions
participant SF as SourceFile
MM->>FW: Initialize code writer instance
MM->>FW: Write models, type definitions, and fields
MM->>EF: Collect additional function declarations
MM->>SF: Merge metadata with extra functions
SF-->>MM: Return final metadata output
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts (1)
240-243
: Consider extracting common writer patterns.The pattern of creating a new
FastWriter
, writing conditionally, and then extracting the result is repeated several times in the file. Consider extracting this into a utility function to reduce duplication.+private generateStatementFromCondition(condition: boolean, trueBlock: () => void, falseBlock: () => void): string { + const writer = new FastWriter(); + if (condition) { + trueBlock(); + } else { + falseBlock(); + } + return writer.result; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/schema/src/plugins/enhancer/policy/expression-writer.ts
(2 hunks)packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts
(24 hunks)packages/schema/src/plugins/enhancer/policy/utils.ts
(9 hunks)packages/sdk/src/code-gen.ts
(1 hunks)packages/sdk/src/model-meta-generator.ts
(14 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/schema/src/plugins/enhancer/policy/expression-writer.ts (1)
packages/sdk/src/code-gen.ts (1)
CodeWriter
(77-83)
packages/sdk/src/model-meta-generator.ts (3)
packages/sdk/src/code-gen.ts (2)
FastWriter
(88-129)CodeWriter
(77-83)packages/language/src/ast.ts (1)
DataModel
(56-66)packages/sdk/src/utils.ts (1)
getAuthDecl
(482-488)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build-test (20.x)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (33)
packages/schema/src/plugins/enhancer/policy/expression-writer.ts (2)
24-25
: Update import to use CodeWriter from SDKThe import has been updated to use
CodeWriter
from '@zenstackhq/sdk' instead of (presumably) the previously usedCodeBlockWriter
. This aligns with the new code generation abstraction introduced in this PR.
80-80
: Constructor parameter type updated to CodeWriterThe constructor parameter type has been changed from
CodeBlockWriter
toCodeWriter
. This is consistent with the overall refactoring across multiple files to improve code generation performance.packages/sdk/src/code-gen.ts (2)
73-83
: Good abstraction for source code generationThe
CodeWriter
interface provides a clean abstraction for code generation with methods for block creation, text writing, and conditional logic. This interface will make it easier to swap out different implementations in the future.
88-129
: Efficient FastWriter implementationThe
FastWriter
class implements theCodeWriter
interface with a straightforward string-based approach that should be more efficient than the previous implementation. The class maintains internal state for indentation and provides methods that correspond to the interface requirements.Key improvements:
- Uses direct string concatenation for better performance
- Simple indentation management
- Clean API with the
result
getter to retrieve generated codepackages/schema/src/plugins/enhancer/policy/utils.ts (5)
4-6
: Import FastWriter from SDKThe import of
FastWriter
from '@zenstackhq/sdk' has been added, which is used in the refactored functions below.
43-43
: Update ts-morph imports for function structureThe import has been updated to include
FunctionDeclarationStructure
andOptionalKind
from ts-morph. This supports the new approach of returning structured function definitions rather than directly writing to source files.
269-286
: Refactor to return function structure instead of writing directlyThe
generateConstantQueryGuardFunction
now returns aFunctionDeclarationStructure
object instead of writing directly to a source file. This is a better separation of concerns that allows for more flexibility in how the function declarations are used.
291-366
: Improved code generation with FastWriterThe function has been refactored to use
FastWriter
and return aFunctionDeclarationStructure
. Key improvements:
- No longer requires a
SourceFile
parameter, simplifying the API- Uses
FastWriter
for generating code blocks- More structured approach with cleaner conditionals for generating code
This change maintains the same functionality while making the code more efficient and maintainable.
433-497
: Refactor entity checker function to return structureSimilar to the previous functions,
generateEntityCheckerFunction
has been updated to return aFunctionDeclarationStructure
instead of writing directly to a source file. This continues the pattern of separating the structure definition from the actual writing process.packages/sdk/src/model-meta-generator.ts (6)
22-27
: Update imports for new code generation approachThe imports have been updated to include
FastWriter
and related structures from ts-morph, supporting the new code generation approach throughout the file.
77-91
: Refactor generate function to use FastWriterThe
generate
function has been improved to:
- Use
FastWriter
for code generation- Collect extra functions in an array
- Add these functions to the source file if present
This change makes the code more structured and maintains a clear separation between code generation and source file manipulation.
101-107
: Update function signature to use CodeWriterThe
generateModelMetadata
function signature has been updated to useCodeWriter
instead ofSourceFile
andCodeBlockWriter
, and to include anextraFunctions
parameter. This simplifies the function and follows the pattern established in other files.
117-122
: Simplify writeModels function signatureThe
writeModels
function no longer requires aSourceFile
parameter and now accepts aCodeWriter
andextraFunctions
array. This is consistent with the overall refactoring approach.
233-237
: Simplify writeFields function signatureThe
writeFields
function now usesCodeWriter
instead ofSourceFile
andCodeBlockWriter
, and includes anextraFunctions
parameter. This continues the pattern of simplification across the codebase.
509-544
: Refactor generateDefaultValueProvider to use extraFunctions arrayThe
generateDefaultValueProvider
function has been updated to:
- No longer require a
SourceFile
parameter- Accept an
extraFunctions
array- Push generated functions to this array instead of directly to the source file
This change improves the function's cohesion by focusing it on generating the function structure rather than modifying the source file.
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts (18)
18-20
: Good implementation of the new abstraction.The switch from directly using
CodeBlockWriter
to the more abstractCodeWriter
interface with a concreteFastWriter
implementation supports better separation of concerns and is consistent with the PR's performance improvement goals.
37-37
: Clean import update.Updated the import to include
OptionalKind
which is needed for the newextraFunctions
array.
54-55
: Good design for function declaration management.Adding the
extraFunctions
array is a clean approach to collect function declarations first and add them to the source file later, which improves code organization.
65-70
: Improved code generation approach.Using
FastWriter
instead of directly writing to the source file centralizes code generation and provides better control over the output.
78-78
: Simplified variable initialization.Directly using
writer.result
for the initializer eliminates unnecessary intermediate variables.
83-85
: Clean separation of concerns.Adding the conditional check for extra functions provides a clean separation between function collection and function insertion into the source file.
126-126
: Consistent method signature simplification.Simplified method signatures by replacing
CodeBlockWriter
withCodeWriter
and removing thesourceFile
parameter, making the code more consistent and less coupled.Also applies to: 148-150, 163-164, 172-173, 184-185, 292-293, 302-303, 323-324, 540-540, 549-549, 610-610
187-188
: Improved function name handling.Now gets the function name from the generation method rather than generating and immediately using the function, which is more maintainable.
232-271
: Effective refactoring of code generation.The
generateCreateInputCheckerFunction
now uses aFastWriter
for statement generation and pushes toextraFunctions
instead of immediately creating function structures. This approach is more consistent with the overall design pattern in the PR.
273-290
: Clean function declaration management.Returning just the function name and storing the function structure in
extraFunctions
simplifies the API and makes function management more centralized.
400-417
: Improved return value structure.The
writeEntityChecker
method now returns a structured object withfunctionName
andselector
, which provides better data organization and makes the API more explicit.
433-440
: Consistent function handling.Now consistently adds generated functions to
extraFunctions
array and references them by name, which aligns with the pattern used throughout the file.
465-467
: Standardized function generation pattern.Follows the standard pattern of generating a function, adding it to
extraFunctions
, and writing the function name to the output.
500-501
: Consistent approach to permission checker functions.Uses the same pattern of getting the function name and writing it, maintaining consistency with other similar method implementations.
520-533
: Effective function declaration refactoring.Changed function declaration to return just the name, with the structure pushed to
extraFunctions
, maintaining consistency with other function generation methods.
565-567
: Consistent pattern application across file.Maintains consistency in how guard functions are generated, stored, and referenced throughout the file.
Also applies to: 626-628, 649-650
674-679
: Clean method refactoring.Simplified the
writeAuthSelector
method by removing thesourceFile
parameter while maintaining the core functionality.
722-741
: Consistent pattern application.The
writeValidationMeta
method follows the same pattern of accepting aCodeWriter
parameter and using it consistently.
fixes #2058 |
No description provided.