refactor!: [ENG-2327] move from deno to node runtime
#239
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
Contributor
Greptile SummaryMigrates the CLI from Deno runtime to Node.js to resolve compatibility issues with interactive terminal libraries ( Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI as sf CLI (Node)
participant Build as Build Pipeline
participant Runtime as Node Runtime
Note over User,Runtime: Development Flow
User->>CLI: bun run dev/prod [command]
CLI->>Runtime: Load Intl.Segmenter polyfill
CLI->>Runtime: Initialize async main()
Runtime->>CLI: Execute command with Node APIs
Note over User,Runtime: Build & Release Flow
User->>Build: Trigger release workflow
Build->>Build: Run biome lint + tsc check
Build->>Build: Run vitest tests
Build->>Build: Bump version in package.json
Build->>Build: tsup: Bundle src → dist/index.cjs
Build->>Build: pkg: Compile to native binaries
Note right of Build: Targets: node22-linux-x64/arm64,<br/>node22-macos-x64/arm64
Build->>Build: Create zip archives
Build->>Build: Create GitHub release
Note over User,Runtime: Installation Flow
User->>CLI: curl install.sh | bash
CLI->>CLI: Detect OS/architecture
CLI->>CLI: Download sf-node22-{platform}-{arch}.zip
CLI->>CLI: Extract to ~/.local/bin/sf
User->>Runtime: sf [command]
Runtime->>User: Execute with bundled dependencies
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Additional Comments (3)
-
src/lib/orders/index.tsx, line 332 (link)syntax: Typo in header key: 'Content-ype' should be 'Content-Type'
-
src/lib/scale/update.tsx, line 248 (link)logic: missing dependencies in
useEffect- should includeupdateProcurements,nodesRequired, and other values used in the effect -
src/lib/zones.tsx, line 153-159 (link)logic: Color functions
cyan,green,red, andgrayare not imported from chalk or defined elsewhere - these will cause runtime errors
77 files reviewed, 12 comments
sigmachirality
changed the title
deno to node runtime
sigmachirality
changed the title
deno to node runtimedeno to node runtime
sigmachirality
force-pushed
the
dt/migrate-to-node
branch
from
1fe4ed3 to
d573efd
Compare
- Add tsconfig.json with bundler moduleResolution for tsx runtime - Restore biome.json from git history for linting/formatting - Add vitest.config.ts for test runner configuration Part of Deno to Node.js migration (Phase 1: Setup)
- Add "type": "module" for ESM support - Add pkg configuration for binary compilation - Update scripts: dev, prod, lint, check, test - Add devDependencies: typescript, tsx, vitest, @biomejs/biome, @yao-pkg/pkg - Add chalk dependency (replaces jsr:@std/fmt/colors) - Add type declarations: @types/cli-progress, @types/async-retry - Add bun.lock for bun package manager Part of Deno to Node.js migration (Phase 1: Setup)
Files updated: - src/helpers/config.ts: Deno.readTextFile -> fs.readFile, etc. - src/helpers/feature-flags.ts: Deno.mkdir -> fs.mkdir, etc. - src/lib/upgrade.ts: Deno.Command -> child_process.spawn - src/lib/clusters/kubeconfig.ts: Deno.errors -> Node.js error codes - src/lib/clusters/keys.tsx: Deno.chmod -> fs.chmod - src/lib/nodes/image/upload.ts: Deno.open -> fs.open, Deno.exit -> process.exit Replaced APIs: - Deno.readTextFile() -> fs.promises.readFile() - Deno.writeTextFile() -> fs.promises.writeFile() - Deno.mkdir() -> fs.promises.mkdir() - Deno.Command -> child_process.spawn() - Deno.errors.NotFound -> err.code === 'ENOENT' Part of Deno to Node.js migration (Phase 3)
Migrated 26 files from Deno's JSR color imports to chalk:
- import { cyan, gray, yellow } from "jsr:@std/fmt/colors"
-> import chalk from "chalk"
- cyan("text") -> chalk.cyan("text")
- brightRed("text") -> chalk.redBright("text")
Part of Deno to Node.js migration (Phase 4)
Updated imports from Deno-specific npm: namespace to standard: - import boxen from "npm:boxen@8.0.1" -> import boxen from "boxen" - import yn from "npm:yn" -> import yn from "yn" - import * as nacl from "npm:tweetnacl" -> import nacl from "tweetnacl" Part of Deno to Node.js migration (Phase 4)
Updated 5 test files:
- src/helpers/test/units.test.ts
- src/helpers/test/duration.test.ts
- src/lib/orders/__tests__/OrderDisplay.test.ts
- src/lib/clusters/kubeconfig.test.ts
- src/lib/clusters/utils.test.ts
Changes:
- Deno.test("name", fn) -> test("name", fn)
- assertEquals(a, b) -> expect(a).toEqual(b)
- import from "jsr:@std/assert" -> import from "vitest"
Part of Deno to Node.js migration (Phase 5)
ci.yml: - Replace denoland/setup-deno with actions/setup-node - Replace deno fmt/lint/check/test with npm run lint/check/test release.yml: - Replace deno setup with Node.js 20 - Replace deno run with npx tsx for release script - Use npm ci for dependency installation Part of Deno to Node.js migration (Phase 6)
- Delete deno.json (Deno runtime configuration) - Delete deno.lock (Deno dependency lock file) - Update .tool-versions to remove deno, keep nodejs 20.19.0 Part of Deno to Node.js migration (Phase 7: Cleanup)
- Replace deno compile with npx pkg for binary compilation - Map pkg targets to original output names for backward compatibility: - node20-linux-x64 -> sf-x86_64-unknown-linux-gnu - node20-linux-arm64 -> sf-aarch64-unknown-linux-gnu - node20-macos-x64 -> sf-x86_64-apple-darwin - node20-macos-arm64 -> sf-aarch64-apple-darwin - Replace Deno.Command with child_process.spawnSync - Replace Deno.readTextFile/writeTextFile with fs.readFileSync/writeFileSync Part of Deno to Node.js migration
- Remove unused @ts-expect-error directives after migration - Add explicit type assertions for response.json() calls - Fix 'unknown' type errors by adding proper type casts Files: me.ts, orders/index.tsx, tokens.ts, posthog.ts, vm/replace.ts
- Remove @ts-expect-error directive in index.ts - Update import organization (biome) - Minor error handling improvements
Auto-fixed by biome check --write: - Organize imports alphabetically - Fix quote style consistency - Remove unused imports - Standardize formatting
Auto-generated schema file reformatted by biome for consistency. No functional changes.
Comprehensive migration plan documenting: - CI workflow updates - Release process changes (deno compile -> pkg) - Auto-upgrade compatibility considerations - Deno-specific packages replaced (jsr:, npm:, Deno.* APIs) - Configuration changes (tsconfig, biome, vitest) - Migration phases and execution order
Fixes tsx warning about unsettled top-level await by wrapping the entire script in an async main() function. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use biomejs/setup-biome@v2 for proper biome installation and run biome ci for optimized CI output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update release script and install.sh to use pkg target names (node20-linux-x64, etc.) instead of mapping to Deno-style names. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace Deno setup instructions with Node.js - Update development commands (npm run dev/prod) - Update CI quality checks (biome, tsc, vitest) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
sigmachirality
force-pushed
the
dt/migrate-to-node
branch
from
f771b3d to
805e709
Compare
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use oven-sh/setup-bun@v2 and bun commands instead of actions/setup-node@v4 with npm. Bun is the package manager. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replaced Deno formatter with Biome for various file types in settings.json and updated extension recommendations accordingly.
- Updated number parsing to use Number.isNaN for clarity. - Streamlined string formatting in various log messages and commands for consistency. - Removed unnecessary type stripping and improved readability in several files.
Member
Author
|
@greptileai review again |
| esbuildOptions(options) { | ||
| options.bundle = true; | ||
| options.alias = { | ||
| "react-devtools-core": "./src/stubs/react-devtools-core.ts", |
Contributor
There was a problem hiding this comment.
syntax: missing stub file
The stub file ./src/stubs/react-devtools-core.ts referenced in this alias doesn't exist in the repository. This will cause the build to fail when esbuild tries to resolve this alias.
Suggested change
| "react-devtools-core": "./src/stubs/react-devtools-core.ts", | |
| // "react-devtools-core": "./src/stubs/react-devtools-core.ts", |
Either create the stub file or remove this alias if it's not needed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tsup.config.ts
Line: 19:19
Comment:
**syntax:** missing stub file
The stub file `./src/stubs/react-devtools-core.ts` referenced in this alias doesn't exist in the repository. This will cause the build to fail when esbuild tries to resolve this alias.
```suggestion
// "react-devtools-core": "./src/stubs/react-devtools-core.ts",
```
Either create the stub file or remove this alias if it's not needed.
How can I resolve this? If you propose a fix, please make it concise.
sigmachirality
changed the title
deno to node runtimedeno to node runtime
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Due to bugs arising from the iteraction of packages like
inquirerandoraand the node-compat functionality ofDenosuch as denoland/deno#30747, we are migrating away fromDenoback toNodefor now.We remain excited by the development of alternate JS runtimes such as
DenoandBun, but are focused on delivering a good UX for our customers as a top priority, of which bugs detract from.Due to: yao-pkg/pkg-fetch#134, we must polyfill Intl.Segmenter using
@formatjs.