fix: Windows compatibility - replace Unix commands with cross-platform alternatives #25377
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@tanuj-ekayaa is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
|
…m alternatives ## Summary Fixes Windows incompatibility issues where Unix-specific shell commands (`rm`, `cp`, `cd`) prevented `yarn install` and `yarn dx` from working on Windows systems. ## Changes - **Replaced `rm -rf` commands** in 9 packages with cross-platform Node.js `fs.rmSync()` and `fs.cpSync()` - **Fixed `@calcom/platform-enums`** build script (removed `rm -rf ./dist`) - **Fixed `@calcom/platform-types`** build script (removed `rm -rf ./dist`) - **Fixed `@calcom/embed-core`** build and clean scripts (replaced Unix commands) - **Fixed `@calcom/embed-react`** build and clean scripts (replaced `rm` and `cp`) - **Fixed `@calcom/embed-snippet`** build and clean scripts - **Fixed `@calcom/prisma`** db-up script to run from correct directory - **Fixed `@calcom/web` and `@calcom/api`** clean scripts - **Removed problematic `.env` symlink** in `packages/prisma/.env` that caused Docker Compose failures - **Added database port mapping** to `docker-compose.yml` (5450:5432) ## Test Plan - Tested `yarn install` completes successfully on Windows 11 - Tested Docker Compose starts database and redis containers - Verified all scripts use cross-platform Node.js APIs - Changes work on Windows, macOS, and Linux (Node.js fs module is cross-platform) ## Related Issues This extends the fix from calcom#14445 which only addressed `@calcom/embed-core`. The same issue existed in platform packages and other embed packages, blocking Windows developers from setting up the project.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 10 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/platform/types/package.json">
<violation number="1" location="packages/platform/types/package.json:8">
Build no longer clears `dist`, so removed/renamed files can linger and be published. Please restore a cross‑platform clean step before running `tsc`.</violation>
</file>
<file name="packages/platform/enums/package.json">
<violation number="1" location="packages/platform/enums/package.json:20">
Dropping the dist cleanup step means stale compiled files stick around when sources are removed or renamed, because `tsc --build` never deletes old outputs. Ensure the build script removes `./dist` in a cross-platform way before running tsc.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
packages/platform/types/package.json
Outdated
| "scripts": { | ||
| "build": "yarn clean && tsc --build --force tsconfig.json", | ||
| "clean": "rm -rf ./dist", | ||
| "build": "tsc --build --force tsconfig.json", |
Contributor
There was a problem hiding this comment.
Build no longer clears dist, so removed/renamed files can linger and be published. Please restore a cross‑platform clean step before running tsc.
Prompt for AI agents
Address the following comment on packages/platform/types/package.json at line 8:
<comment>Build no longer clears `dist`, so removed/renamed files can linger and be published. Please restore a cross‑platform clean step before running `tsc`.</comment>
<file context>
@@ -5,8 +5,7 @@
"scripts": {
- "build": "yarn clean && tsc --build --force tsconfig.json",
- "clean": "rm -rf ./dist",
+ "build": "tsc --build --force tsconfig.json",
"build:watch": "tsc --build --force ./tsconfig.json --watch",
"post-install": "yarn build"
</file context>
Suggested change
| "build": "tsc --build --force tsconfig.json", | |
| "build": "node -e \"require('fs').rmSync('./dist',{recursive:true,force:true})\" && tsc --build --force tsconfig.json", |
✅ Addressed in 8424f07
packages/platform/enums/package.json
Outdated
| "scripts": { | ||
| "test": "jest ./tests", | ||
| "build": "rm -rf ./dist && tsc --build --force tsconfig.json", | ||
| "build": "tsc --build --force tsconfig.json", |
Contributor
There was a problem hiding this comment.
Dropping the dist cleanup step means stale compiled files stick around when sources are removed or renamed, because tsc --build never deletes old outputs. Ensure the build script removes ./dist in a cross-platform way before running tsc.
Prompt for AI agents
Address the following comment on packages/platform/enums/package.json at line 20:
<comment>Dropping the dist cleanup step means stale compiled files stick around when sources are removed or renamed, because `tsc --build` never deletes old outputs. Ensure the build script removes `./dist` in a cross-platform way before running tsc.</comment>
<file context>
@@ -17,7 +17,7 @@
"scripts": {
"test": "jest ./tests",
- "build": "rm -rf ./dist && tsc --build --force tsconfig.json",
+ "build": "tsc --build --force tsconfig.json",
"build:watch": "tsc --build --force ./tsconfig.json --watch",
"post-install": "yarn build"
</file context>
Suggested change
| "build": "tsc --build --force tsconfig.json", | |
| "build": "node -e \"require('fs').rmSync('./dist', { recursive: true, force: true });\" && tsc --build --force tsconfig.json", |
✅ Addressed in 8424f07
- Add explicit dist cleanup before tsc build in platform-enums - Add explicit dist cleanup before tsc build in platform-types - Addresses cubic-dev-ai review: tsc --build --force doesn't remove stale files - Uses Node.js fs.rmSync for cross-platform compatibility
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (reviewed changes from recent commits).
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/platform/types/package.json">
<violation number="1" location="packages/platform/types/package.json:8">
After deleting ./dist, the build needs to force TypeScript to re-emit outputs; otherwise the stale tsconfig.tsbuildinfo makes `tsc --build` skip compilation and leaves dist empty.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| "scripts": { | ||
| "build": "yarn clean && tsc --build --force tsconfig.json", | ||
| "clean": "rm -rf ./dist", | ||
| "build": "node -e \"require('fs').rmSync('./dist',{recursive:true,force:true})\" && tsc --build tsconfig.json", |
Contributor
There was a problem hiding this comment.
After deleting ./dist, the build needs to force TypeScript to re-emit outputs; otherwise the stale tsconfig.tsbuildinfo makes tsc --build skip compilation and leaves dist empty.
Prompt for AI agents
Address the following comment on packages/platform/types/package.json at line 8:
<comment>After deleting ./dist, the build needs to force TypeScript to re-emit outputs; otherwise the stale tsconfig.tsbuildinfo makes `tsc --build` skip compilation and leaves dist empty.</comment>
<file context>
@@ -5,7 +5,7 @@
"private": true,
"scripts": {
- "build": "tsc --build --force tsconfig.json",
+ "build": "node -e \"require('fs').rmSync('./dist',{recursive:true,force:true})\" && tsc --build tsconfig.json",
"build:watch": "tsc --build --force ./tsconfig.json --watch",
"post-install": "yarn build"
</file context>
Suggested change
| "build": "node -e \"require('fs').rmSync('./dist',{recursive:true,force:true})\" && tsc --build tsconfig.json", | |
| "build": "node -e \"require('fs').rmSync('./dist',{recursive:true,force:true})\" && tsc --build --force tsconfig.json", |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes Windows incompatibility issues where Unix-specific shell commands (
rm,cp,cd) preventedyarn installandyarn dxfrom working on Windows systems.Fixes #25376
Changes Made
rm -rfcommands in 9 packages with cross-platform Node.jsfs.rmSync()andfs.cpSync()@calcom/platform-enumsbuild script (removedrm -rf ./dist)@calcom/platform-typesbuild script (removedrm -rf ./dist)@calcom/embed-corebuild and clean scripts (replaced Unix commands)@calcom/embed-reactbuild and clean scripts (replacedrmandcp)@calcom/embed-snippetbuild and clean scripts@calcom/prismadb-up script to run from correct directory@calcom/weband@calcom/apiclean scripts.envsymlink inpackages/prisma/.envthat caused Docker Compose failuresdocker-compose.yml(5450:5432)Test Plan
yarn installcompletes successfully on Windows 11Related Issues
This extends the fix from #14445 which only addressed
@calcom/embed-core. The same issue existed in platform packages and other embed packages, blocking Windows developers from setting up the project.Additional Context