Skip to content

chore(ts): cleanup ts scripts#307

Open
stalniy wants to merge 1 commit into
mainfrom
fix/sync-sdl-schema
Open

chore(ts): cleanup ts scripts#307
stalniy wants to merge 1 commit into
mainfrom
fix/sync-sdl-schema

Conversation

@stalniy
Copy link
Copy Markdown
Contributor

@stalniy stalniy commented May 15, 2026

📝 Description

SDL's json schema was changed but validator in ts was not regenerated

🔧 Purpose of the Change

  • New feature implementation
  • Bug fix
  • Documentation update
  • Code refactoring
  • Dependency upgrade
  • Other: [specify]

📌 Related Issues

  • Closes #ISSUE_NUMBER
  • References #ISSUE_NUMBER

✅ Checklist

  • I've updated relevant documentation
  • Code follows Akash Network's style guide
  • I've added/updated relevant unit tests
  • Dependencies have been properly updated
  • I agree and adhered to the Contribution Guidelines

📎 Notes for Reviewers

[Include any additional context, architectural decisions, or specific areas to focus on]

@stalniy stalniy requested a review from a team as a code owner May 15, 2026 04:33
@github-actions github-actions Bot added the C:ts label May 15, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Review Change Stack

Walkthrough

This PR removes experimental Node.js flags (--experimental-strip-types --no-warnings) from an npm script in package.json and from shebangs across six TypeScript utility scripts. All Node.js invocations now use plain node without these experimental runtime options.

Changes

Node.js script flag removal

Layer / File(s) Summary
Remove experimental flags from npm scripts and shebangs
ts/package.json, ts/script/compile-json-schema-to-ts.ts, ts/script/fix-ts-proto-generated-types.ts, ts/script/protoc-gen-customtype-patches.ts, ts/script/protoc-gen-sdk-object.ts, ts/script/protoc-gen-type-index-files.ts, ts/script/validate-package-exports.ts
The compile:validators npm script and shebangs in all build and validation utility scripts remove --experimental-strip-types --no-warnings Node.js flags, executing with plain node instead.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 Experimental flags take flight,
Seven scripts now plain and light,
Node runs true, no warnings dance,
Simplicity in every glance! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title 'chore(ts): cleanup ts scripts' is partially related to the changeset. While it describes the nature of changes (cleanup of TypeScript scripts), it does not clearly communicate the main objective: removing experimental Node.js flags from script shebangs. The title is overly broad and lacks specificity about what cleanup entails. Consider a more specific title that describes the actual change, such as 'chore(ts): remove experimental flags from script shebangs' or 'refactor(ts): simplify node shebang invocations'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description provides context about the bug fix and indicates relevant sections were completed, though Related Issues section lacks specific issue numbers.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sync-sdl-schema

Comment @coderabbitai help to get the list of available commands and usage tips.

baktun14
baktun14 previously approved these changes May 26, 2026
@stalniy stalniy force-pushed the fix/sync-sdl-schema branch from 2e1b716 to e9fce1b Compare May 26, 2026 21:23
@stalniy stalniy changed the title fix(ts): sync sdl schema validation chore(ts): cleanup ts scripts May 26, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ts/package.json`:
- Line 43: Update the package.json engines.node range to require Node >=22.18.0
so native TypeScript execution works for the scripts that run TS files directly;
specifically, change the engines.node field to raise the lower bound (for
example to ">=22.18.0 <25") so running scripts like the "compile:validators"
script ("node ./script/compile-json-schema-to-ts.ts") and
"./script/validate-package-exports.ts" won't need --experimental-strip-types
(volta.node can remain pinned at 24.14.1).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cd4fcdda-5eb9-413f-acdf-c464a3940b48

📥 Commits

Reviewing files that changed from the base of the PR and between 2e1b716 and e9fce1b.

📒 Files selected for processing (7)
  • ts/package.json
  • ts/script/compile-json-schema-to-ts.ts
  • ts/script/fix-ts-proto-generated-types.ts
  • ts/script/protoc-gen-customtype-patches.ts
  • ts/script/protoc-gen-sdk-object.ts
  • ts/script/protoc-gen-type-index-files.ts
  • ts/script/validate-package-exports.ts
✅ Files skipped from review due to trivial changes (1)
  • ts/script/protoc-gen-type-index-files.ts

Comment thread ts/package.json
"test:unit": "vitest run --project unit",
"test:sdk-compatibility": "vitest run --project functional sdl-parity.spec.ts",
"compile:validators": "node --experimental-strip-types --no-warnings ./script/compile-json-schema-to-ts.ts",
"compile:validators": "node ./script/compile-json-schema-to-ts.ts",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Current engine range:"
rg -n '"node":\s*".*"' ts/package.json

echo
echo "Node guidance (expect: < v22.18.0 needs --experimental-strip-types):"
curl -fsSL https://nodejs.org/en/learn/typescript/run-natively \
  | rg -n 'less than v22\.18\.0|--experimental-strip-types'

echo
echo "Node TypeScript history (expect: enabled by default from v22.18.0):"
curl -fsSL https://nodejs.org/api/typescript.html \
  | rg -n 'v23\.6\.0, v22\.18\.0|enabled by default'

Repository: akash-network/chain-sdk

Length of output: 2075


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# show the relevant scripts/sections with line numbers
sed -n '30,130p' ts/package.json | cat -n

echo
echo "Occurrences of node running .ts files in scripts:"
rg -n '"scripts"\s*:\s*\{' -n ts/package.json && true
rg -n 'node\s+.*\.ts' ts/package.json || true

echo
echo "compile:validators entry:"
rg -n '"compile:validators"\s*:' ts/package.json

Repository: akash-network/chain-sdk

Length of output: 3382


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Search for experimental-strip-types / experimental-transform-types in ts/:"
rg -n --hidden --no-ignore-vcs 'experimental-strip-types|experimental-transform-types' ts || true

echo
echo "Find script files referenced by ts/package.json:"
fd -t f 'compile-json-schema-to-ts\.ts' -d 4 . || true
fd -t f 'validate-package-exports\.ts' -d 4 . || true

echo
echo "Inspect first ~60 lines of compile-json-schema-to-ts.ts (if found):"
f="$(fd -t f 'compile-json-schema-to-ts\.ts' -d 4 . -E node_modules | head -n 1 || true)"
if [ -n "${f:-}" ]; then
  echo "File: $f"
  head -n 60 "$f" | nl -ba
else
  echo "compile-json-schema-to-ts.ts not found"
fi

echo
echo "Inspect first ~60 lines of validate-package-exports.ts (if found):"
f="$(fd -t f 'validate-package-exports\.ts' -d 4 . -E node_modules | head -n 1 || true)"
if [ -n "${f:-}" ]; then
  echo "File: $f"
  head -n 60 "$f" | nl -ba
else
  echo "validate-package-exports.ts not found"
fi

echo
echo "Search for node invocations that run .ts files anywhere under ts/:"
rg -n 'node\s+[^#\n]*\.ts\b' ts || true

echo
echo "Search for shebangs in .ts files under script/:"
fd -t f '\.ts$' ts/script -d 4 | while read -r p; do
  head -n 1 "$p" | rg -n '^#!' >/dev/null 2>&1 && echo "Shebang in $p: $(head -n 1 "$p" | tr -d '\n')"
done || true

Repository: akash-network/chain-sdk

Length of output: 552


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Search for experimental-strip-types / experimental-transform-types in ts/:"
rg -n --hidden --no-ignore-vcs 'experimental-strip-types|experimental-transform-types' ts || true

echo
echo "Inspect ts/script/compile-json-schema-to-ts.ts (first 120 lines):"
f="./ts/script/compile-json-schema-to-ts.ts"
if [ -f "$f" ]; then
  sed -n '1,120p' "$f" | cat -n
else
  echo "Missing $f"
fi

echo
echo "Inspect ts/script/validate-package-exports.ts (first 120 lines):"
f="./ts/script/validate-package-exports.ts"
if [ -f "$f" ]; then
  sed -n '1,120p' "$f" | cat -n
else
  echo "Missing $f"
fi

echo
echo "Search for node invocations that run .ts files under ts/:"
rg -n 'node\s+[^#\n]*\.ts\b' ts || true

echo
echo "Search for any direct node execution of script/*.ts with flags (e.g., --experimental-strip-types) under ts/:"
rg -n 'node\s+--[^#\n]*\.ts\b' ts || true

Repository: akash-network/chain-sdk

Length of output: 9579


Raise engines.node to >=22.18.0 to match native TypeScript execution

ts/package.json runs node ./script/compile-json-schema-to-ts.ts and ./script/validate-package-exports.ts (both have #!/usr/bin/env -S node and include TS-only syntax like type annotations/assertions). Node’s native TS “type stripping” is enabled by default starting in v22.18.0; for < v22.18.0, running TS files directly requires --experimental-strip-types. With engines.node still set to >=22.14.0 <25, Node 22.14–22.17 can fail at build time.

🔧 Proposed fix
   "engines": {
-    "node": ">=22.14.0 <25"
+    "node": ">=22.18.0 <25"
   },

volta.node is already pinned to 24.14.1, so only the engines.node range needs adjustment.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ts/package.json` at line 43, Update the package.json engines.node range to
require Node >=22.18.0 so native TypeScript execution works for the scripts that
run TS files directly; specifically, change the engines.node field to raise the
lower bound (for example to ">=22.18.0 <25") so running scripts like the
"compile:validators" script ("node ./script/compile-json-schema-to-ts.ts") and
"./script/validate-package-exports.ts" won't need --experimental-strip-types
(volta.node can remain pinned at 24.14.1).

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants