-
-
Notifications
You must be signed in to change notification settings - Fork 95
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
feat(hooks): add "portable" generation mode #1850
Conversation
📝 WalkthroughWalkthroughThe pull request introduces multiple enhancements across various files, primarily focusing on improving export capabilities, error handling, and code quality. Key changes include the addition of new export statements and functions, modifications to existing function signatures, and the removal of ESLint disabling comments to promote better code practices. The changes also include the introduction of new variables for managing Prisma client paths and type definitions, as well as the addition of new test cases to validate functionality. Overall, the modifications aim to streamline the codebase while maintaining existing functionalities. 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: 5
🧹 Outside diff range and nitpick comments (13)
packages/schema/src/plugins/enhancer/index.ts (1)
29-29
: Consider consistent naming between destructured and returned propertiesThe variable is destructured as
newPrismaClientDtsPath
but returned asprismaClientDtsPath
. Consider using consistent naming to improve code maintainability.- const { dmmf, newPrismaClientDtsPath } = await new EnhancerGenerator(model, options, project, outDir).generate(); + const { dmmf, prismaClientDtsPath } = await new EnhancerGenerator(model, options, project, outDir).generate();packages/sdk/src/code-gen.ts (1)
43-43
: Consider optimizing file saving performanceThe current implementation mixes synchronous and asynchronous operations and processes files sequentially. This could be improved for better performance.
Consider this optimization:
export async function saveProject(project: Project) { - project.getSourceFiles().forEach(saveSourceFile); + await Promise.all(project.getSourceFiles().map(saveSourceFile)); await project.save(); }This change would:
- Process files in parallel
- Maintain consistent async/await usage
- Properly propagate errors from individual file operations
packages/sdk/src/types.ts (1)
34-39
: Consider enhancing the documentation for portable mode.The new
prismaClientDtsPath
property is well-structured and consistent with the codebase. However, since this is part of the portable generation feature, consider enhancing the documentation to explain its role in that context./** - * PrismaClient's TypeScript declaration file's path + * Path to PrismaClient's TypeScript declaration file (.d.ts). + * Used when generating portable hooks to ensure proper TypeScript support + * when the hooks are exported as standalone modules. * @private */ prismaClientDtsPath?: string;packages/plugins/tanstack-query/tests/portable.test.ts (2)
112-152
: Enhance test coverage for inheritance and delegationThe test covers basic inheritance, but should verify that the generated hooks properly handle inherited fields and delegation.
Consider adding these test cases:
it('supports portable for logical client', async () => { + const result = await loadSchema( // ... existing code ... ); + + // Verify type inheritance + const hookContent = result.getFileContent('hooks/index.ts'); + expect(hookContent).toContain('createdAt: Date'); + expect(hookContent).toContain('type: string'); + + // Add test for delegate behavior + it('handles delegate directive correctly', async () => { + const { useFindUniqueUser } = require(result.getFilePath('hooks/index.ts')); + expect(useFindUniqueUser.type).toBe('User'); + }); });
7-153
: Improve test structure and reduce duplicationThe test suite has several areas of duplicate code that could be extracted to improve maintainability.
Consider these improvements:
- Extract common configuration:
const commonConfig = { provider: 'postgresql', pushDb: false, extraDependencies: ['react@18.2.0', '@types/react@18.2.0', '@tanstack/react-query@5.56.x'], compile: true, };
- Move test file content to fixtures:
// __fixtures__/sample-hooks.ts export const basicUsage = ` import { useFindUniqueUser } from './hooks'; const { data } = useFindUniqueUser({ where: { id: 1 } }); console.log(data?.email); `;
- Add shared setup/teardown:
describe('Tanstack Query Plugin Portable Tests', () => { let tmpDir: tmp.DirResult; beforeEach(() => { tmpDir = tmp.dirSync({ unsafeCleanup: true }); }); afterEach(() => { tmpDir.removeCallback(); }); // ... tests ... });packages/schema/src/plugins/prisma/index.ts (1)
84-95
: Consider enhancing error handling and path resolution.While the type declaration path resolution works, there are opportunities for improvement:
- The error message could be more specific about why resolution failed
- Consider checking multiple common locations for type declarations
Here's a suggested enhancement:
try { const prismaClientResolvedPath = require.resolve(clientOutputDir, { - paths: [path.dirname(options.schemaPath)], + paths: [ + path.dirname(options.schemaPath), + process.cwd(), + path.join(process.cwd(), 'node_modules'), + ], }); prismaClientDtsPath = path.join(path.dirname(prismaClientResolvedPath), 'index.d.ts'); } catch (err) { console.warn( colors.yellow( - `Could not resolve PrismaClient type declaration path. This may break plugins that depend on it.` + `Could not resolve PrismaClient type declaration path from ${clientOutputDir}. ` + + `Reason: ${err instanceof Error ? err.message : 'unknown'}. ` + + `This may break plugins that depend on it. ` + + `Ensure @prisma/client is installed and the path is correct.` ) ); }packages/plugins/swr/src/generator.ts (2)
Line range hint
39-63
: Consider decomposing generateModelHooks for better maintainabilityThe function handles multiple responsibilities (queries, mutations, aggregations). Consider breaking it down into smaller, focused functions for better maintainability and testability.
Suggested structure:
function generateModelHooks(...) { setupImports(sf); generateBasicCrudHooks(sf, model, mapping); generateAggregationHooks(sf, model, mapping); generateCustomHooks(sf, model); } function generateBasicCrudHooks(sf: SourceFile, model: DataModel, mapping: DMMF.ModelMapping) { // Handle create, update, delete operations } function generateAggregationHooks(sf: SourceFile, model: DataModel, mapping: DMMF.ModelMapping) { // Handle aggregate, groupBy, count operations } function generateCustomHooks(sf: SourceFile, model: DataModel) { // Handle check and other custom operations }
Line range hint
16-24
: Enhance error handling with structured warningsThe current warning system could be improved to provide more context and severity levels.
Consider using a structured warning system:
type WarningLevel = 'error' | 'warning' | 'info'; interface GeneratorWarning { level: WarningLevel; message: string; model?: string; operation?: string; suggestion?: string; } // Usage in the code: if (!mapping) { warnings.push({ level: 'error', message: `Unable to find mapping for model ${dataModel.name}`, model: dataModel.name, suggestion: 'Ensure the model is properly defined in your schema' }); return; }packages/schema/src/plugins/enhancer/enhance/index.ts (3)
Line range hint
449-507
: Consider caching regex patterns for better performance.The
fixJsonFieldType
method creates new RegExp instances in each iteration. Consider caching these patterns:private fixJsonFieldType(typeAlias: TypeAliasDeclaration, source: string) { + // Cache regex patterns + private static readonly JSON_FIELD_REGEX = (fieldName: string) => + new RegExp(`(${fieldName}\\??\\s*):[^\\n]+`); + private static readonly TYPE_PATTERNS = [ + 'GroupByOutputType', + '(Unchecked)?Create(\\S+?)?Input', + '(Unchecked)?Update(\\S+?)?Input', + 'CreateManyInput', + '(Unchecked)?UpdateMany(Mutation)?Input', + ]; // ... rest of the method - return source.replace( - new RegExp(`(${field.name}\\??\\s*):[^\\n]+`), + return source.replace( + EnhancerGenerator.JSON_FIELD_REGEX(field.name), `$1: ${field.type.reference!.$refText}${field.type.array ? '[]' : ''}${ field.type.optional ? ' | null' : '' }` );
Line range hint
4-4
: Add test coverage for the enhanced type generation.The TODO comment indicates missing tests. Consider adding unit tests to verify:
- Correct type generation for delegate models
- Proper handling of discriminator fields
- JSON field type transformations
Would you like me to help create a test suite for these scenarios?
Line range hint
293-321
: Improve temporary file cleanup handling.The cleanup of temporary files could be more robust:
- Consider using
try-finally
to ensure cleanup- Add error logging for cleanup failures
private async generateLogicalPrisma() { const logicalPrismaFile = path.join(zmodelDir, `logical-${Date.now()}.prisma`); + try { // ... existing generation code ... - try { - // clean up temp schema - if (fs.existsSync(logicalPrismaFile)) { - fs.rmSync(logicalPrismaFile); - } - } catch { - // ignore errors - } return { prismaSchema: logicalPrismaFile, dmmf, }; + } finally { + try { + if (fs.existsSync(logicalPrismaFile)) { + fs.rmSync(logicalPrismaFile); + } + } catch (error) { + console.warn(`Failed to clean up temporary schema file: ${error}`); + } + } }packages/schema/src/plugins/zod/transformer.ts (2)
Line range hint
1-1000
: Consider splitting the Transformer class into smaller, focused classes.The
Transformer
class has multiple responsibilities including:
- Schema generation
- File I/O operations
- Import management
- Type transformations
Consider breaking it down into smaller, focused classes following the Single Responsibility Principle:
SchemaGenerator
: Handle schema generation logicFileWriter
: Handle file I/O operationsImportManager
: Handle import statementsTypeTransformer
: Handle type transformationsThis would improve maintainability, testability, and make the code easier to understand.
Line range hint
17-24
: Consider removing static state and following immutable patterns.The class maintains static state through
enumNames
,rawOpsMap
, andoutputPath
, which could lead to issues:
- Static state makes the class harder to test
- Potential issues in concurrent scenarios
- Difficulty in maintaining state across different instances
Consider:
- Moving static state to instance variables
- Making the class follow immutable patterns
- Using dependency injection for shared state
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/sdk/package.json
is excluded by!**/*.json
packages/sdk/src/package.json
is excluded by!**/*.json
📒 Files selected for processing (15)
packages/plugins/swr/src/generator.ts
(1 hunks)packages/plugins/tanstack-query/src/generator.ts
(6 hunks)packages/plugins/tanstack-query/tests/portable.test.ts
(1 hunks)packages/plugins/trpc/src/client-helper/index.ts
(0 hunks)packages/plugins/trpc/src/generator.ts
(0 hunks)packages/schema/src/cli/plugin-runner.ts
(3 hunks)packages/schema/src/plugins/enhancer/enhance/index.ts
(2 hunks)packages/schema/src/plugins/enhancer/index.ts
(2 hunks)packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts
(0 hunks)packages/schema/src/plugins/prisma/index.ts
(3 hunks)packages/schema/src/plugins/zod/generator.ts
(2 hunks)packages/schema/src/plugins/zod/transformer.ts
(3 hunks)packages/sdk/src/code-gen.ts
(2 hunks)packages/sdk/src/model-meta-generator.ts
(0 hunks)packages/sdk/src/types.ts
(2 hunks)
💤 Files with no reviewable changes (4)
- packages/plugins/trpc/src/client-helper/index.ts
- packages/plugins/trpc/src/generator.ts
- packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts
- packages/sdk/src/model-meta-generator.ts
🔇 Additional comments (17)
packages/schema/src/plugins/enhancer/index.ts (1)
47-47
: Verify the impact of the new return property
The addition of prismaClientDtsPath
to the return object looks good and aligns with the PR objective of supporting portable hooks. However, we should verify that all consumers of this plugin function are prepared to handle this new property.
✅ Verification successful
Based on the search results, particularly the PluginResult
type definition from packages/sdk/src/types.ts
, I can now generate the final response:
The new return property is safe to add
The prismaClientDtsPath
property is being added to a return object that implements the PluginResult
type. This type is defined as an open interface that only requires warnings
array and makes prismaClientPath
optional. Adding new optional properties to this return object is safe and won't break existing consumers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing usages of the plugin's return value to ensure compatibility
# Search for direct destructuring of the plugin's return value
rg -A 2 "const \{.*\} = .*run\("
# Search for any assignments of the plugin's return value
rg -A 2 "= .*run\("
# Search for type definitions that might need updating
ast-grep --pattern 'type $_ = { $$$, prismaClientPath: $_ }'
Length of output: 119
Script:
#!/bin/bash
# Let's try a different approach to find usages of the plugin's return value
# Search for files importing or using the enhancer plugin
rg -l "from.*plugins/enhancer"
# Look for any function calls that might be consuming the plugin's output
rg -A 5 "enhancer\("
# Search for type definitions related to the plugin
rg -A 5 "interface.*Plugin"
rg -A 5 "type.*Plugin"
# Search specifically for the run function usage
rg -A 5 "\.run\("
Length of output: 14519
Script:
#!/bin/bash
# Let's check the PluginResult type definition and its consumers
# Search for the PluginResult type definition
rg -A 10 "export.*type.*PluginResult"
# Search for files that might be consuming the plugin's return value
rg -l "packages/schema/src/plugins/enhancer"
# Look for direct usage of the enhancer plugin's run function
rg -A 5 "from.*plugins/enhancer.*run"
rg -A 5 "import.*plugins/enhancer.*run"
Length of output: 701
packages/sdk/src/code-gen.ts (1)
1-2
: Verify the package.json import path
The import path ./package.json
seems unusual for an SDK package. Typically, it would be relative to the root package (e.g., '../package.json').
✅ Verification successful
Let me verify the content of the package.json in that directory to ensure it's the correct one being imported.
The ./package.json
import path is correct
The import path is correct as the package.json exists in the same directory as code-gen.ts. This is a valid setup as evidenced by:
- The package.json exists at
packages/sdk/src/package.json
- It's the correct package.json for the SDK package (
@zenstackhq/sdk
) - The package explicitly exports its package.json via the
exports
field:"./package.json": "./package.json"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the package.json location relative to code-gen.ts
# Find the location of code-gen.ts
cd_path=$(fd --type f "code-gen.ts$" --exec dirname {} \;)
# Check if package.json exists in the same directory
if [ -f "${cd_path}/package.json" ]; then
echo "package.json exists in the same directory"
else
echo "package.json not found in the same directory"
# Look for package.json in parent directory
if [ -f "${cd_path}/../package.json" ]; then
echo "package.json found in parent directory"
fi
fi
Length of output: 220
Script:
#!/bin/bash
# Check the content of package.json in the same directory as code-gen.ts
cd_path=$(fd --type f "code-gen.ts$" --exec dirname {} \;)
cat "${cd_path}/package.json"
Length of output: 1726
packages/sdk/src/types.ts (1)
34-39
: Verify if @private
annotation is appropriate.
Both properties are marked with @private
annotations, but they appear to be integral to the portable generation feature which is a public-facing functionality according to the PR objectives. Please verify if these should remain private or if they should be part of the public API.
Also applies to: 83-88
✅ Verification successful
@private
annotation is appropriate and should be kept
The @private
annotation for prismaClientDtsPath
is correct for the following reasons:
- It's an internal implementation detail used by plugins (like tanstack-query) to locate TypeScript declaration files
- The property is not part of the user-facing API as evidenced by its usage pattern
- It's used alongside other correctly marked private properties like
dmmf
andshortNameMap
- The property is primarily used for internal type generation and bundling purposes, as seen in the tanstack-query generator
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if these properties are used in public-facing APIs or documentation
# Search for references to prismaClientDtsPath in documentation and examples
echo "Searching for documentation references..."
rg -g '*.md' -g '*.mdx' 'prismaClientDtsPath'
# Search for usage in test files to understand the intended API surface
echo "Searching for test usage..."
rg -g '*test*' -g '*spec*' 'prismaClientDtsPath'
# Search for usage in implementation files
echo "Searching for implementation usage..."
rg -g '*.ts' -g '*.js' --type-not test 'prismaClientDtsPath'
Length of output: 427
Script:
#!/bin/bash
# Let's try a different approach to search for usage patterns
# Search for prismaClientDtsPath in all files
echo "Searching for all references to prismaClientDtsPath..."
rg "prismaClientDtsPath"
# Look for the interface or type definition containing this property
echo "Looking for the type definition..."
ast-grep --pattern 'interface $_ {
$$$
prismaClientDtsPath?: $_
$$$
}'
# Search for any exports of the containing type/interface
echo "Checking if the containing type is exported..."
rg "export.*prismaClientDtsPath" -A 5 -B 5
Length of output: 1793
Script:
#!/bin/bash
# Let's examine the type definition and its usage context
# Find the interface/type containing this property and its export status
echo "Checking the type definition context..."
rg -B 10 -A 10 "prismaClientDtsPath\?:" "packages/sdk/src/types.ts"
# Check how this property is used in the tanstack-query generator
echo "Checking usage in tanstack-query generator..."
rg -B 10 -A 10 "prismaClientDtsPath" "packages/plugins/tanstack-query/src/generator.ts"
# Look for any public API documentation or exports
echo "Checking for exports and documentation..."
ast-grep --pattern 'export interface $_ {
$$$
prismaClientDtsPath?: $_
$$$
}'
Length of output: 2644
packages/plugins/tanstack-query/tests/portable.test.ts (1)
1-6
: LGTM! Well-structured test file setup
The imports and setup are well-organized, using appropriate testing utilities and following best practices for test isolation.
packages/schema/src/plugins/prisma/index.ts (3)
31-39
: LGTM: Clear variable declarations with good documentation.
The new variables are well-structured with clear purposes and appropriate default values.
79-82
: LGTM: Proper handling of custom client output paths.
The code correctly handles both absolute and relative paths, maintaining consistency with the client location for portable usage.
110-110
: Verify return type definitions are updated.
The return statement correctly includes the new prismaClientDtsPath
, but we should verify that the corresponding type definitions are updated.
✅ Verification successful
Type definitions are properly updated
The type definitions are correctly updated in the codebase. The prismaClientDtsPath
property is properly defined as an optional string property in the relevant type definitions in packages/sdk/src/types.ts
and is being used consistently across the codebase in the plugin system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for type definitions of the plugin function return type
ast-grep --pattern 'type $returnType = { $$$, prismaClientDtsPath$$$, $$$ }'
# Search for any references to the plugin function return type
rg -l "prismaClientDtsPath" --type ts
Length of output: 322
Script:
#!/bin/bash
# Let's check the type definitions in the files that reference prismaClientDtsPath
rg "prismaClientDtsPath" -A 5 -B 5 --type ts
# Specifically look for interface or type definitions
ast-grep --pattern 'interface $name {
$$$
prismaClientDtsPath$$$
$$$
}'
ast-grep --pattern 'type $name = {
$$$
prismaClientDtsPath$$$
$$$
}'
Length of output: 9282
packages/plugins/swr/src/generator.ts (1)
262-262
: LGTM: Metadata export enhances hook portability
The addition of metadata export aligns well with the PR's objective of implementing a "portable" generation mode. This change makes the hooks more self-contained by ensuring that model metadata is available when hooks are exported and used in other repositories.
Let's verify that the metadata export is consistent across other plugin packages:
✅ Verification successful
Metadata export is consistently implemented across relevant plugins
The verification shows that the metadata export pattern is correctly implemented in both SWR and Tanstack Query plugins, which are the data fetching plugins that require model metadata. Both plugins:
- Export metadata from
__model_meta
- Include the
generateModelMeta
functionality
This consistency confirms that the metadata export addition is part of a well-coordinated implementation of the portable generation mode across relevant plugins.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if metadata export is consistently implemented across plugin packages
# Expected: Similar metadata export patterns in other plugin generators
# Check for metadata exports in other plugin generators
rg -l "export.*metadata.*from.*__model_meta" "packages/plugins/*/src/generator.ts"
# Verify metadata file generation
rg -l "generateModelMeta" "packages/plugins/*/src/generator.ts"
Length of output: 300
Script:
#!/bin/bash
# Let's try a different approach by first finding the plugin directories
fd -t d "plugins" packages/
# Then search for generator files
fd "generator.ts" packages/plugins/
# Now let's check for metadata exports and generation
rg -l "export.*metadata.*from.*__model_meta" packages/plugins/
rg -l "generateModelMeta" packages/plugins/
# Let's also check the content of the current file to understand the context better
cat packages/plugins/swr/src/generator.ts
Length of output: 16633
packages/schema/src/cli/plugin-runner.ts (3)
139-140
: LGTM: Variable declaration follows established patterns
The declaration of prismaClientDtsPath
is well-placed alongside other Prisma-related variables and follows consistent typing patterns.
179-179
: Consider making prismaClientDtsPath non-optional in PluginOptions
The type assertion as PluginOptions
suggests that prismaClientDtsPath
might be required by some plugins. Consider making this property non-optional in the PluginOptions
type if it's essential for the portable generation mode.
#!/bin/bash
# Search for plugins that require prismaClientDtsPath
rg -l "prismaClientDtsPath.*\?" --type ts
Also applies to: 185-185
170-170
: Verify plugin result type definition includes prismaClientDtsPath
The code assumes the plugin result contains prismaClientDtsPath
. Let's verify this property is properly defined in the plugin result type.
✅ Verification successful
Property prismaClientDtsPath is properly defined in PluginResult type
The verification confirms that prismaClientDtsPath
is correctly defined as an optional string property in the PluginResult
type in packages/sdk/src/types.ts
. The property is properly documented and used consistently across the codebase, particularly in the plugin system for handling PrismaClient's TypeScript declaration files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for PluginResult type definition and verify it includes prismaClientDtsPath
ast-grep --pattern 'type PluginResult = {
$$$
prismaClientDtsPath?: $_
$$$
}'
Length of output: 219
Script:
#!/bin/bash
# Search for files containing prismaClientDtsPath
rg -A 5 -B 5 'prismaClientDtsPath' --type ts
# Also search for PluginResult type definition
rg -A 10 -B 2 'type\s+PluginResult' --type ts
Length of output: 11814
packages/schema/src/plugins/zod/generator.ts (2)
16-16
: LGTM!
The addition of saveSourceFile
import is properly placed and aligns with the changes in file saving logic.
147-147
: 🛠️ Refactor suggestion
Consider error handling in the file saving logic
The switch from Promise.all
to forEach
might affect error handling. With forEach
, if one file fails to save, the subsequent files might still be processed without proper error propagation.
Let's verify the error handling implementation in the saveSourceFile
function:
Consider wrapping the save operation in a try-catch block or using Promise.all for better error handling:
- this.sourceFiles.forEach(saveSourceFile);
+ try {
+ await Promise.all(this.sourceFiles.map(saveSourceFile));
+ } catch (error) {
+ throw new Error(`Failed to save generated files: ${error.message}`);
+ }
packages/schema/src/plugins/enhancer/enhance/index.ts (1)
Line range hint 65-136
: LGTM: Method signature enhancement properly handles type definitions path.
The updated return type and implementation correctly handle the path to the fixed Prisma client type definitions. The conditional path calculation ensures that the path is only returned when types are actually fixed.
packages/schema/src/plugins/zod/transformer.ts (1)
62-63
: LGTM! Improved code quality by removing ESLint disable comments.
The removal of ESLint disable comments while maintaining functionality indicates better code quality and compliance with linting rules.
Also applies to: 75-76, 110-110
packages/plugins/tanstack-query/src/generator.ts (2)
73-75
: Conditional generation of bundled types based on 'portable' option
The generateBundledTypes
function is correctly called when the portable
option is enabled, ensuring that bundled types are generated when needed.
598-598
: Adding 'metadata' export in index file
The export statement correctly exposes metadata
from './__model_meta'
, allowing consumers to access the model metadata.
if (options.portable && typeof options.portable !== 'boolean') { | ||
throw new PluginError( | ||
name, | ||
`Invalid value for "portable" option: ${options.portable}, a boolean value is expected` | ||
); | ||
} |
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.
Ensure 'portable' option validation handles all non-boolean values
The current validation if (options.portable && typeof options.portable !== 'boolean')
may not catch falsey non-boolean values like 0
or ''
. To accurately validate the portable
option, consider checking if it is defined and not a boolean.
Apply this diff to fix the condition:
-if (options.portable && typeof options.portable !== 'boolean') {
+if (typeof options.portable !== 'undefined' && typeof options.portable !== 'boolean') {
📝 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.
if (options.portable && typeof options.portable !== 'boolean') { | |
throw new PluginError( | |
name, | |
`Invalid value for "portable" option: ${options.portable}, a boolean value is expected` | |
); | |
} | |
if (typeof options.portable !== 'undefined' && typeof options.portable !== 'boolean') { | |
throw new PluginError( | |
name, | |
`Invalid value for "portable" option: ${options.portable}, a boolean value is expected` | |
); | |
} |
fix #1759