Преглед изворни кода

fix(cli): Replace regex solution with robust stirp-json-comments package (#3694)

Co-authored-by: Housein Abo Shaar <76689341+GogoIsProgramming@users.noreply.github.com>
Housein Abo Shaar пре 6 месеци
родитељ
комит
26a9e0395c

+ 161 - 0
packages/cli/e2e/cli-migrate.e2e-spec.ts

@@ -0,0 +1,161 @@
+import { afterEach, beforeAll, describe, expect, it } from 'vitest';
+
+import { CliTestProject, createProblematicTsConfig, createTestProject } from './cli-test-utils';
+
+describe('CLI Migrate Command E2E', () => {
+    let testProject: CliTestProject;
+
+    beforeAll(() => {
+        try {
+            // eslint-disable-next-line @typescript-eslint/no-var-requires
+            const { execSync } = require('child_process');
+            execSync('npm run build', { cwd: __dirname + '/..', stdio: 'inherit' });
+        } catch (error) {
+            throw new Error('Failed to build CLI before running e2e tests. Run "npm run build" first.');
+        }
+    });
+
+    afterEach(() => {
+        if (testProject) {
+            testProject.cleanup();
+        }
+    });
+
+    describe('tsconfig.json comment stripping', () => {
+        it('should successfully parse tsconfig.json with /* patterns in path values', async () => {
+            testProject = createTestProject('tsconfig-comment-test');
+
+            // Create a tsconfig.json with the problematic patterns that broke the old implementation
+            testProject.writeFile('tsconfig.json', createProblematicTsConfig());
+
+            // Try to run a CLI command that loads the tsconfig.json
+            // Since the migrate command is currently not exposed, we'll test the underlying functionality
+            // by using the --help command which should still load the config for validation
+            const result = await testProject.runCliCommand(['--help']);
+
+            // The command should complete successfully without crashing
+            expect(result.exitCode).toBe(0);
+            expect(result.stdout).toContain('Usage:');
+        });
+
+        it('should handle tsconfig.json with various comment patterns', async () => {
+            testProject = createTestProject('complex-tsconfig-test');
+
+            // Create a more complex tsconfig.json with various comment patterns
+            const complexTsConfig = `{
+    // Single line comment
+    "compilerOptions": {
+        /* Multi-line comment at the beginning */
+        "target": "ES2021",
+        "module": "commonjs", // Inline comment
+        "baseUrl": "./",
+        /*
+         * Multi-line comment block
+         * with multiple lines
+         */
+        "paths": {
+            // This path contains /* which was problematic
+            "@/vdb/*": ["./node_modules/@vendure/dashboard/src/lib/*"],
+            "comment": "This // looks like a comment but isn't",
+            "multiComment": "This /* looks */ like /* multiple */ comments",
+            "protocol": "file://some-protocol-url/*",
+            "regex": "some/*/pattern/*/here"
+        }
+        /* Final comment */
+    }
+    // End comment
+}`;
+
+            testProject.writeFile('tsconfig.json', complexTsConfig);
+
+            // The CLI should handle this complex config without issues
+            const result = await testProject.runCliCommand(['--help']);
+            expect(result.exitCode).toBe(0);
+        });
+
+        it('should preserve path mappings with wildcards correctly', async () => {
+            testProject = createTestProject('wildcard-paths-test');
+
+            // Create a tsconfig with various wildcard patterns
+            const wildcardTsConfig = `{
+    "compilerOptions": {
+        "baseUrl": "./",
+        "paths": {
+            "@/dashboard/*": ["./node_modules/@vendure/dashboard/src/lib/*"],
+            "@/plugins/*/src": ["./src/plugins/*/src/index.ts"],
+            "@/api/*/types": ["./src/api/*/types.ts"],
+            "*/utils": ["./src/utils/*"],
+            "glob/**": ["./src/glob/**/*"]
+        }
+    }
+}`;
+
+            testProject.writeFile('tsconfig.json', wildcardTsConfig);
+
+            // Test that the CLI doesn't crash with these patterns
+            const result = await testProject.runCliCommand(['--help']);
+            expect(result.exitCode).toBe(0);
+        });
+    });
+
+    describe('CLI command execution', () => {
+        it('should show help when no arguments provided', async () => {
+            testProject = createTestProject('help-test');
+
+            // CLI shows help but exits with code 1 when no arguments provided
+            const result = await testProject.runCliCommand([], { expectError: true });
+            expect(result.exitCode).toBe(1);
+            expect(result.stderr).toContain('Usage:');
+        });
+
+        it('should show version when --version flag is used', async () => {
+            testProject = createTestProject('version-test');
+
+            const result = await testProject.runCliCommand(['--version']);
+            expect(result.exitCode).toBe(0);
+            expect(result.stdout).toMatch(/\d+\.\d+\.\d+/); // Should contain version number
+        });
+
+        it('should handle invalid commands gracefully', async () => {
+            testProject = createTestProject('invalid-command-test');
+
+            const result = await testProject.runCliCommand(['invalid-command'], { expectError: true });
+            expect(result.exitCode).not.toBe(0);
+            expect(result.stderr).toContain('unknown command');
+        });
+    });
+
+    describe('project structure validation', () => {
+        it('should work with minimal project structure', async () => {
+            testProject = createTestProject('minimal-project');
+
+            // Create minimal files
+            testProject.writeFile(
+                'tsconfig.json',
+                JSON.stringify(
+                    {
+                        compilerOptions: {
+                            target: 'ES2021',
+                            module: 'commonjs',
+                        },
+                    },
+                    null,
+                    2,
+                ),
+            );
+
+            const result = await testProject.runCliCommand(['--help']);
+            expect(result.exitCode).toBe(0);
+        });
+
+        it('should handle missing tsconfig.json gracefully', async () => {
+            testProject = createTestProject('no-tsconfig');
+
+            // Don't create a tsconfig.json file
+            const result = await testProject.runCliCommand(['--help']);
+
+            // Should still work, as tsconfig.json is not always required for help command
+            expect(result.exitCode).toBe(0);
+        });
+    });
+});

+ 198 - 0
packages/cli/e2e/cli-test-utils.ts

@@ -0,0 +1,198 @@
+import { spawn } from 'child_process';
+import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs';
+import { tmpdir } from 'os';
+import { join } from 'path';
+
+export interface CliTestProject {
+    projectDir: string;
+    cleanup: () => void;
+    writeFile: (relativePath: string, content: string) => void;
+    readFile: (relativePath: string) => string;
+    fileExists: (relativePath: string) => boolean;
+    runCliCommand: (
+        args: string[],
+        options?: { expectError?: boolean },
+    ) => Promise<{ stdout: string; stderr: string; exitCode: number }>;
+}
+
+/**
+ * Creates a temporary test project for CLI testing
+ */
+export function createTestProject(projectName: string = 'test-project'): CliTestProject {
+    const projectDir = join(
+        tmpdir(),
+        'vendure-cli-e2e',
+        `${projectName}-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`,
+    );
+
+    // Create project directory
+    mkdirSync(projectDir, { recursive: true });
+
+    // Create basic package.json
+    const packageJson = {
+        name: projectName,
+        version: '1.0.0',
+        private: true,
+        dependencies: {
+            '@vendure/core': '3.3.7',
+            '@vendure/common': '3.3.7',
+        },
+        devDependencies: {
+            typescript: '5.8.2',
+        },
+    };
+
+    writeFileSync(join(projectDir, 'package.json'), JSON.stringify(packageJson, null, 2));
+
+    // Create basic vendure-config.ts
+    const vendureConfig = `
+import { VendureConfig } from '@vendure/core';
+
+export const config: VendureConfig = {
+    apiOptions: {
+        port: 3000,
+        adminApiPath: 'admin-api',
+        shopApiPath: 'shop-api',
+    },
+    authOptions: {
+        tokenMethod: ['bearer', 'cookie'],
+        superadminCredentials: {
+            identifier: 'superadmin',
+            password: 'superadmin',
+        },
+        cookieOptions: {
+            secret: 'cookie-secret',
+        },
+    },
+    dbConnectionOptions: {
+        type: 'sqlite',
+        database: ':memory:',
+        synchronize: true,
+    },
+};
+`;
+
+    writeFileSync(join(projectDir, 'vendure-config.ts'), vendureConfig);
+
+    return {
+        projectDir,
+        cleanup: () => {
+            try {
+                rmSync(projectDir, { recursive: true, force: true });
+            } catch (error) {
+                // Ignore cleanup errors - use stderr to avoid no-console lint error
+                const errorMessage = error instanceof Error ? error.message : String(error);
+                process.stderr.write(`Failed to cleanup test project at ${projectDir}: ${errorMessage}\n`);
+            }
+        },
+        writeFile: (relativePath: string, content: string) => {
+            const fullPath = join(projectDir, relativePath);
+            const dir = join(fullPath, '..');
+            mkdirSync(dir, { recursive: true });
+            writeFileSync(fullPath, content);
+        },
+        readFile: (relativePath: string) => {
+            return readFileSync(join(projectDir, relativePath), 'utf-8');
+        },
+        fileExists: (relativePath: string) => {
+            return existsSync(join(projectDir, relativePath));
+        },
+        runCliCommand: async (args: string[], options: { expectError?: boolean } = {}) => {
+            return new Promise((resolve, reject) => {
+                // Use the built CLI from the dist directory
+                const cliPath = join(__dirname, '..', 'dist', 'cli.js');
+
+                const child = spawn('node', [cliPath, ...args], {
+                    cwd: projectDir,
+                    env: {
+                        ...process.env,
+                        // Ensure we don't inherit any CLI environment variables
+                        VENDURE_RUNNING_IN_CLI: undefined,
+                    },
+                    stdio: ['pipe', 'pipe', 'pipe'],
+                });
+
+                let stdout = '';
+                let stderr = '';
+
+                child.stdout?.on('data', data => {
+                    stdout += data.toString();
+                });
+
+                child.stderr?.on('data', data => {
+                    stderr += data.toString();
+                });
+
+                child.on('close', code => {
+                    const exitCode = code ?? 0;
+
+                    if (!options.expectError && exitCode !== 0) {
+                        reject(new Error(`CLI command failed with exit code ${exitCode}. stderr: ${stderr}`));
+                    } else {
+                        resolve({ stdout, stderr, exitCode });
+                    }
+                });
+
+                child.on('error', error => {
+                    reject(error);
+                });
+
+                // Send empty stdin to handle any prompts (for non-interactive testing)
+                child.stdin?.end();
+            });
+        },
+    };
+}
+
+/**
+ * Creates a tsconfig.json with the problematic patterns that caused the original bug
+ */
+export function createProblematicTsConfig(): string {
+    return `{
+    // TypeScript configuration file with comments
+    "compilerOptions": {
+        "target": "ES2021",
+        "module": "commonjs",
+        "lib": ["ES2021"],
+        "outDir": "./dist",
+        "rootDir": "./src",
+        "baseUrl": "./",
+        /* Path mappings that contain /* patterns in strings - this was breaking the old regex implementation */
+        "paths": {
+            "@/vdb/*": ["./node_modules/@vendure/dashboard/src/lib/*"],
+            "@/components/*": ["src/components/*"],
+            "@/api/*": ["./api/*/index.ts"],
+            "@/plugins/*": ["./src/plugins/*/src/index.ts"],
+            "special": "path/with/*/wildcards"
+        },
+        "strict": true,
+        "esModuleInterop": true,
+        "skipLibCheck": true,
+        "forceConsistentCasingInFileNames": true,
+        "experimentalDecorators": true,
+        "emitDecoratorMetadata": true
+    },
+    "include": ["src/**/*", "vendure-config.ts"],
+    "exclude": ["node_modules", "dist"]
+}`;
+}
+
+/**
+ * Waits for a condition to be true with timeout
+ */
+export async function waitFor(
+    condition: () => boolean | Promise<boolean>,
+    timeoutMs: number = 5000,
+    intervalMs: number = 100,
+): Promise<void> {
+    const startTime = Date.now();
+
+    while (Date.now() - startTime < timeoutMs) {
+        if (await condition()) {
+            return;
+        }
+        await new Promise(resolve => setTimeout(resolve, intervalMs));
+    }
+
+    throw new Error(`Condition not met within ${timeoutMs}ms`);
+}

+ 6 - 1
packages/cli/package.json

@@ -22,7 +22,9 @@
         "build": "rimraf dist && tsc -p ./tsconfig.cli.json && ts-node ./build.ts",
         "watch": "tsc -p ./tsconfig.cli.json --watch",
         "ci": "npm run build",
-        "test": "vitest --config vitest.config.mts --run"
+        "test": "vitest --config vitest.config.mts --run",
+        "e2e": "cross-env PACKAGE=cli vitest --config ../../e2e-common/vitest.config.mts --run",
+        "e2e:watch": "cross-env PACKAGE=cli vitest --config ../../e2e-common/vitest.config.mts"
     },
     "publishConfig": {
         "access": "public"
@@ -41,12 +43,15 @@
         "dotenv": "^16.4.5",
         "fs-extra": "^11.2.0",
         "picocolors": "^1.0.0",
+        "strip-json-comments": "^5.0.2",
         "ts-morph": "^21.0.1",
         "ts-node": "^10.9.2",
         "tsconfig-paths": "^4.2.0"
     },
     "devDependencies": {
         "@vendure/core": "3.3.7",
+        "@vendure/testing": "3.3.7",
+        "cross-env": "^7.0.3",
         "typescript": "5.8.2"
     }
 }

+ 134 - 0
packages/cli/src/commands/migrate/load-vendure-config-file.spec.ts

@@ -0,0 +1,134 @@
+import stripJsonComments from 'strip-json-comments';
+import { describe, expect, it } from 'vitest';
+
+describe('stripJsonComments', () => {
+    it('should handle JSON with /* in string values correctly', () => {
+        const input = `{
+            "compilerOptions": {
+                "paths": {
+                    "@/vdb/*": ["./node_modules/@vendure/dashboard/src/lib/*"]
+                }
+            }
+        }`;
+
+        const result = stripJsonComments(input);
+        const parsed = JSON.parse(result);
+
+        expect(parsed.compilerOptions.paths['@/vdb/*']).toEqual([
+            './node_modules/@vendure/dashboard/src/lib/*',
+        ]);
+    });
+
+    it('should remove actual comments while preserving /* in strings', () => {
+        const input = `{
+            // This is a comment
+            "compilerOptions": {
+                /* Multi-line comment */
+                "paths": {
+                    "@/vdb/*": ["./node_modules/@vendure/dashboard/src/lib/*"],
+                    "test": "value with /* inside string"
+                }
+            }
+        }`;
+
+        const result = stripJsonComments(input);
+        const parsed = JSON.parse(result);
+
+        expect(parsed.compilerOptions.paths['@/vdb/*']).toEqual([
+            './node_modules/@vendure/dashboard/src/lib/*',
+        ]);
+        expect(parsed.compilerOptions.paths.test).toBe('value with /* inside string');
+    });
+
+    it('should handle complex paths with wildcards and comments', () => {
+        const input = `{
+            "compilerOptions": {
+                "baseUrl": "./",
+                // Comment about paths
+                "paths": {
+                    "@/*": ["src/*"],
+                    "@/components/*": ["src/components/*"],
+                    "@/vendor/*": ["./node_modules/@vendure/*/src/*"],
+                    "special": "path/with/*/wildcards"
+                }
+            }
+        }`;
+
+        const result = stripJsonComments(input);
+        const parsed = JSON.parse(result);
+
+        expect(parsed.compilerOptions.paths['@/vendor/*']).toEqual(['./node_modules/@vendure/*/src/*']);
+        expect(parsed.compilerOptions.paths.special).toBe('path/with/*/wildcards');
+    });
+
+    it('should handle nested /* patterns in strings', () => {
+        const input = `{
+            "test": {
+                "pattern1": "/* comment inside string */",
+                "pattern2": "start /* middle */ end",
+                "pattern3": "multiple /* one */ and /* two */"
+            }
+        }`;
+
+        const result = stripJsonComments(input);
+        const parsed = JSON.parse(result);
+
+        expect(parsed.test.pattern1).toBe('/* comment inside string */');
+        expect(parsed.test.pattern2).toBe('start /* middle */ end');
+        expect(parsed.test.pattern3).toBe('multiple /* one */ and /* two */');
+    });
+
+    it('should handle // patterns in strings', () => {
+        const input = `{
+            // Real comment
+            "url": "https://example.com/path",
+            "comment": "This // is not a comment"
+        }`;
+
+        const result = stripJsonComments(input);
+        const parsed = JSON.parse(result);
+
+        expect(parsed.url).toBe('https://example.com/path');
+        expect(parsed.comment).toBe('This // is not a comment');
+    });
+});
+
+describe('JSON parsing with comments', () => {
+    it('should successfully parse tsconfig.json content with /* in paths', () => {
+        const tsConfigContent = `{
+            "compilerOptions": {
+                "baseUrl": "./",
+                "paths": {
+                    "@/vdb/*": ["./node_modules/@vendure/dashboard/src/lib/*"]
+                }
+            }
+        }`;
+
+        // This should not throw an error when parsing
+        const result = JSON.parse(stripJsonComments(tsConfigContent));
+        expect(result.compilerOptions.paths['@/vdb/*']).toEqual([
+            './node_modules/@vendure/dashboard/src/lib/*',
+        ]);
+    });
+
+    it('should handle tsconfig.json with comments and /* in paths', () => {
+        const tsConfigContent = `{
+            // TypeScript configuration
+            "compilerOptions": {
+                "baseUrl": "./",
+                /* Path mappings for module resolution */
+                "paths": {
+                    "@/vdb/*": ["./node_modules/@vendure/dashboard/src/lib/*"],
+                    "@/components/*": ["src/components/*"]
+                }
+            }
+        }`;
+
+        // This should not throw an error when parsing
+        const result = JSON.parse(stripJsonComments(tsConfigContent));
+        expect(result.compilerOptions.paths['@/vdb/*']).toEqual([
+            './node_modules/@vendure/dashboard/src/lib/*',
+        ]);
+        expect(result.compilerOptions.paths['@/components/*']).toEqual(['src/components/*']);
+    });
+});

+ 1 - 7
packages/cli/src/commands/migrate/load-vendure-config-file.ts

@@ -1,19 +1,13 @@
 import { VendureConfig } from '@vendure/core';
 import { readFileSync } from 'node:fs';
 import path from 'node:path';
+import stripJsonComments from 'strip-json-comments';
 import { register } from 'ts-node';
 
 import { VendureConfigRef } from '../../shared/vendure-config-ref';
 import { selectTsConfigFile } from '../../utilities/ast-utils';
 import { isRunningInTsNode } from '../../utilities/utils';
 
-function stripJsonComments(jsonString: string): string {
-    return jsonString
-        .replace(/\/\*[\s\S]+?\*\//g, '')
-        .replace(/\/\/.*$/gm, '')
-        .replace(/^\s*$[\r\n]/gm, '');
-}
-
 export async function loadVendureConfigFile(
     vendureConfig: VendureConfigRef,
     providedTsConfigPath?: string,