-
-
Notifications
You must be signed in to change notification settings - Fork 288
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
fix(prisma): change usr/bin/env ts-node by node #2929
Conversation
WalkthroughThe changes in this pull request involve modifying the shebang line in the Changes
Assessment against linked issues
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
🧹 Outside diff range comments (1)
packages/orm/prisma/src/generator.ts (1)
Import path should be updated to use
.ts
extensionThe import path should reference the TypeScript source file since:
- The TypeScript source exists at
packages/orm/prisma/src/cli/prismaGenerator.ts
- No JavaScript file was found in the codebase
- The tsconfig shows Node.js module resolution which supports TypeScript imports
🔗 Analysis chain
Line range hint
6-6
: Verify import path compatibilityThe
.js
extension in the import path./cli/prismaGenerator.js
suggests this file is meant to be executed as compiled JavaScript.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify import path resolution # Check if the JS file exists or is generated fd prismaGenerator.js # Look for related TypeScript source fd prismaGenerator.ts # Check for any extension-related configuration in tsconfig fd tsconfig.json --exec jq -r '.compilerOptions | select(.moduleResolution != null) | {moduleResolution, allowJs, resolveJsonModule}'Length of output: 473
🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
packages/orm/prisma/src/generator.ts
(1 hunks)
🔇 Additional comments (2)
packages/orm/prisma/src/generator.ts (2)
1-1
: Verify the execution environment setup
The change from ts-node
to node
implies this file should be compiled before execution. Let's verify the build configuration and execution context.
✅ Verification successful
The shebang change is appropriate and properly configured
The change from ts-node
to node
is correct because:
- The generator script is compiled to JavaScript before execution, as evidenced by the bin configuration in package.json pointing to
lib/esm/generator.js
- The package has proper build setup with
build:ts
script and TypeScript configuration - The generator is distributed and executed as a compiled JavaScript file, not as TypeScript source
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check build configuration and related scripts
# Check if there's a build step in package.json
fd package.json --exec jq -r '.scripts | select(.build != null) | .build'
# Check if the file is included in tsconfig
fd tsconfig.json --exec jq -r '.include[]?' | grep -i "generator.ts"
# Look for build output configuration
fd tsconfig.json --exec jq -r '.compilerOptions.outDir'
Length of output: 2712
Script:
#!/bin/bash
# Let's check specifically in the prisma package directory
fd package.json --base-directory packages/orm/prisma --exec jq -r '.scripts'
# Check if there's a specific tsconfig for prisma
fd tsconfig.json --base-directory packages/orm/prisma --exec cat
# Check if the generator is imported or used in other files
rg -l "generator\.ts" packages/orm/prisma
# Check how the generator is referenced in package.json
fd package.json --base-directory packages/orm/prisma --exec jq -r '.bin'
Length of output: 2167
Line range hint 1-16
: Investigate relationship with Prisma injection issue
While the shebang change appears straightforward, we need to understand how this relates to the reported Prisma object injection issue (#2927). The generator script is responsible for schema generation, which could indirectly affect how Prisma objects are injected.
Consider documenting the relationship between this generator change and the Prisma injection mechanism, particularly:
- How the generator output affects Repository class initialization
- The role of this change in fixing the Prisma object injection
🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
36c735f
to
c6f5a8f
Compare
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
packages/orm/prisma/scripts/backup-index.d.mts
(1 hunks)packages/orm/prisma/src/generator.ts
(1 hunks)
🔇 Additional comments (2)
packages/orm/prisma/src/generator.ts (2)
1-1
: LGTM! Verify build process.
The change from ts-node
to node
is appropriate, but we need to ensure TypeScript compilation is handled correctly in the build pipeline.
Let's verify the build setup:
✅ Verification successful
LGTM! Build process is properly configured.
The TypeScript compilation is properly handled in the build pipeline:
- The project uses
tsc
for TypeScript compilation viabuild:ts
scripts - The Prisma package specifically has a build setup that includes TypeScript compilation:
"build": "rm -rf lib && yarn build:ts && monorepo build-hybrid"
- The TypeScript configuration is present in the project (
tsconfig.json
)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify TypeScript compilation setup
# Check for build scripts in package.json
fd "package.json" --exec jq '.scripts | select(. | tostring | contains("build"))'
# Check for TypeScript configuration
fd "tsconfig.json"
Length of output: 20302
Line range hint 1-18
: Verify Prisma generator functionality.
Since this change is related to the Prisma v8 upgrade issue where Repository classes weren't getting Prisma injected properly, we should verify that the generator still produces correct output.
Let's check the generator setup:
✅ Verification successful
The previous results show the generator configurations in schema files, but we need to verify the actual generator implementation to ensure it correctly handles Repository generation. This follow-up script will help us examine the generator's implementation details.
The previous results show repository generation logic and snapshots, but we need to verify the complete generator implementation and how PrismaService is being injected. This will help us confirm if the generator properly handles the Prisma v8 injection concerns mentioned in the review.
The previous commands didn't yield results, so let's try to directly examine the generator implementation file and PrismaService-related files to verify the Prisma v8 injection functionality.
Generator functionality verified and properly handles Prisma injection
The generator implementation correctly handles Prisma v8 integration:
- PrismaService is properly generated with correct dependency injection using
@Injectable()
and@Inject()
- The service extends PrismaClient and implements lifecycle hooks (OnInit, OnDestroy)
- The generator correctly sets up the output paths and prisma client integration
- Repository generation includes proper PrismaService injection as seen in the snapshots
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Prisma generator configuration and output
# Check Prisma schema for generator configuration
fd "schema.prisma" --exec grep -A 5 "generator"
# Check for Repository class generation patterns
ast-grep --pattern 'class $_ extends Repository<$_> {
$$$
}'
Length of output: 2505
Script:
#!/bin/bash
# Let's check the actual generator implementation and output patterns
# Check the generator implementation
ast-grep --pattern 'export async function generate($_) {
$$$
}'
# Check for Repository template or generation logic
rg -A 5 "Repository" packages/orm/prisma/src/
Length of output: 3233
Script:
#!/bin/bash
# Let's examine the actual generator implementation and PrismaService injection
rg -A 10 "export const generate" packages/orm/prisma/src/
# Check the test files to understand expected behavior
rg -A 10 "describe.*Repository" packages/orm/prisma/src/
# Look for PrismaService injection patterns
ast-grep --pattern '@Inject()
protected prisma: PrismaService'
Length of output: 183
Script:
#!/bin/bash
# Let's try a different approach to examine the generator implementation
cat packages/orm/prisma/src/cli/prismaGenerator.js || cat packages/orm/prisma/src/cli/prismaGenerator.ts
# Check for any PrismaService related files
fd "PrismaService" --exec cat {}
# Look for generate function with different pattern
ast-grep --pattern 'function generate({$$$}) {
$$$
}'
Length of output: 8842
🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
🎉 This PR is included in version 8.3.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Closes: #2927
Summary by CodeRabbit