Skip to content

[create-twenty-app] Use vite config lib#16273

Merged
martmull merged 2 commits intomainfrom
vite-config-create-twenty-app
Dec 3, 2025
Merged

[create-twenty-app] Use vite config lib#16273
martmull merged 2 commits intomainfrom
vite-config-create-twenty-app

Conversation

@prastoin
Copy link
Copy Markdown
Contributor

@prastoin prastoin commented Dec 3, 2025

Followup #16258

@prastoin prastoin marked this pull request as ready for review December 3, 2025 10:29
@socket-security
Copy link
Copy Markdown

socket-security bot commented Dec 3, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​apollo/​client@​3.13.8971009198100
Added@​aws-sdk/​client-sts@​3.825.09810010097100
Added@​aws-sdk/​client-sesv2@​3.888.09810010097100
Added@​aws-sdk/​client-lambda@​3.825.09810010097100
Added@​aws-sdk/​client-s3@​3.825.09810010097100

View full report

Comment on lines +68 to +72
output: [
{
format: 'es',
entryFileNames: (chunk) => entryFileNames(chunk, 'mjs'),
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: ES module output cli.mjs uses __dirname, which is undefined in ESM, causing ReferenceError on import.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The vite.config.ts generates an ES module output (cli.mjs) that includes references to __dirname. In ES module contexts, __dirname is not defined, leading to a ReferenceError: __dirname is not defined when cli.mjs is imported. This makes the ES module version of the package unusable for consumers using ES module syntax, despite the CommonJS version working correctly.

💡 Suggested Fix

Configure Vite/Rollup to either polyfill __dirname for ES module output, or use import.meta.url for path resolution in cli.mjs and app-template.ts. Alternatively, remove the ES module export if it's not intended for ES module consumption.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/create-twenty-app/vite.config.ts#L68-L72

Potential issue: The `vite.config.ts` generates an ES module output (`cli.mjs`) that
includes references to `__dirname`. In ES module contexts, `__dirname` is not defined,
leading to a `ReferenceError: __dirname is not defined` when `cli.mjs` is imported. This
makes the ES module version of the package unusable for consumers using ES module
syntax, despite the CommonJS version working correctly.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 5129230

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use import.meta.dirname

@martmull martmull enabled auto-merge (squash) December 3, 2025 10:34
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Dec 3, 2025

Greptile Overview

Greptile Summary

Migrates create-twenty-app package from TypeScript Compiler to Vite build system for improved bundling and module output. The migration includes proper ES module and CommonJS dual exports, TypeScript project references, and simplified build configuration.

Key Changes:

  • Added Vite configuration with custom entry handling and asset copying plugin
  • Updated build outputs from .js to .cjs/.mjs extensions with proper package.json exports
  • Refactored TypeScript config to use project references (tsconfig.lib.json, tsconfig.spec.json)
  • Simplified NX build targets by removing custom shell commands in favor of Vite
  • Updated __dirname path in app-template.ts from ../ to ./ to match new bundle structure
  • Added Yarn 4.9.2 release files to base application template

Issues Found:

  • Critical: project.json start command references old dist/cli.js instead of new dist/cli.cjs
  • Minor grammar errors in vite.config.ts error messages ("occurs" should be "occur")

Confidence Score: 3/5

  • This PR has a critical bug that will break the start command, but the core migration logic is sound
  • Score reflects one critical runtime bug in project.json (incorrect file reference) and minor grammar issues in error messages. The Vite migration itself is well-structured with proper path adjustments and bundle configuration, but the broken start command must be fixed before merging.
  • packages/create-twenty-app/project.json requires immediate attention to fix the start command file reference

Important Files Changed

File Analysis

Filename Score Overview
packages/create-twenty-app/vite.config.ts 3/5 New Vite build configuration added with custom entry handling and asset copying. Contains grammar errors in error messages ("occurs" should be "occur").
packages/create-twenty-app/package.json 5/5 Updated to use Vite build system with proper ES module and CommonJS exports. Build script simplified and main/bin paths updated to .cjs extension.
packages/create-twenty-app/project.json 2/5 Simplified build configuration by removing custom commands. Critical bug: start command still references old cli.js instead of cli.cjs.
packages/create-twenty-app/tsconfig.json 5/5 Refactored to use TypeScript project references with separate lib and spec configs, following modern best practices.
packages/create-twenty-app/src/utils/app-template.ts 5/5 Path updated from '../constants/base-application' to './constants/base-application' to match new Vite bundle structure where code is bundled at dist root.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Build as Build System
    participant Vite as Vite
    participant FS as File System
    participant CLI as CLI Runtime

    Dev->>Build: npm run build
    Build->>Vite: Execute vite build
    
    Note over Vite: Compile TypeScript sources
    Vite->>Vite: Bundle src/cli.ts → dist/cli.cjs + dist/cli.mjs
    Vite->>Vite: Apply tsconfigPaths plugin
    Vite->>Vite: Generate types with dts plugin
    
    Note over Vite: Execute copy-assets plugin
    Vite->>FS: Copy src/constants/base-application
    FS->>FS: Create dist/constants/base-application
    
    Note over Vite: Build complete
    
    Dev->>CLI: node dist/cli.cjs [directory]
    CLI->>CLI: Initialize commander program
    CLI->>FS: Read package.json from __dirname/../package.json
    CLI->>CLI: Execute CreateAppCommand
    CLI->>FS: Copy from __dirname/./constants/base-application
    FS->>FS: Create new app directory with template
    CLI->>Dev: App created successfully
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. packages/create-twenty-app/project.json, line 24 (link)

    logic: The build output changed from cli.js to cli.cjs, but the start command still references the old filename

9 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@martmull martmull merged commit a7c5969 into main Dec 3, 2025
70 checks passed
@martmull martmull deleted the vite-config-create-twenty-app branch December 3, 2025 10:47
@twenty-eng-sync
Copy link
Copy Markdown

Hey @prastoin! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

1 similar comment
@twenty-eng-sync
Copy link
Copy Markdown

Hey @prastoin! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants