Sandbox::https://github.com/Kong/volcano-cli/pull/482

swkeever · 2026-11-12T16:55:34Z

The CLI now finds .tsx function handlers and uploads them as source, so the Volcano build compiles their JSX, as it does for .ts, .mts and .cts since #428.

Why

Volcano is adding .tsx handlers to the build: JSX compiled with the function's tsconfig settings, React's automatic runtime by default (Kong/volcano-hosting, .tsx function handlers). Today the CLI skips hello.tsx and hello/index.tsx, so a function that renders HTML with react-dom/server never deploys from volcano functions deploy, and the docs have to tell users the CLI doesn't find .tsx files. Without this, .tsx deploys only through Git.

Discovery

The rules stay in internal/function/typescript.go and match the server's:

<name>.tsx under volcano/functions is a Node.js function. It uploads as index.tsx, never index.js, so the build compiles it.
.tsx ranks after every other entry, so it never changes what an existing function deploys. A directory's index.tsx is its entry only when it has no index.js, main.py, main.rb, index.ts, index.mts or index.cts. hello.ts beside hello.tsx, and index.ts beside index.tsx, deploy the .ts file as they do today; they are not duplicates.
Functions sharing a name rank, lowest first: a JavaScript, Python or Ruby source; a directory with a .ts, .mts or .cts index; such a file; a directory with an index.tsx; a .tsx file. A directory whose entry is JavaScript beside a TypeScript index, such as index.mjs beside index.tsx, ranks as one with a .ts index, ahead of any TypeScript file, because the server ranks its JavaScript entry first. Kong/volcano-hosting#3356 ranks them the same way on the server.
Two TypeScript files of one rank, such as hello.ts and hello.mts, still stop the scan. A directory with two indexes of one rank, such as index.ts and index.mts, uploads, and the build refuses it, as today. The scan never rejects a directory, so one function's layout never blocks deploying another.

User-facing behavior change

A top-level <name>.tsx file or a directory with an index.tsx used to be ignored and now deploys.
One layout deploys differently: a hello/ directory with index.mjs or index.cjs beside index.tsx now deploys that JavaScript instead of a hello.ts file, as Git deploy does. Earlier CLIs didn't find that directory. docs/functions.md says so.
Every other function that deploys today deploys the same file on the same runtime, including one with a .tsx file beside its .ts handler.

User-facing surfaces

CLI: volcano functions deploy, volcano durable deploy and volcano deploy find .tsx handlers, in cloud and local mode (this PR). functions deploy --help names the TypeScript extensions.
Docs: docs/functions.md here. The hosting docs drop "The CLI doesn't find .tsx files" in a follow-up PR once this ships.
Agent skills: Kong/volcano-skills volcano-functions gets the .tsx guidance and the new CLI version in a follow-up PR once this ships.
API, MCP, manifest, SDKs, dashboard: unchanged. The server-side .tsx build is the hosting PR. The dashboard's deploy dialog writes a single index.ts and is out of scope.

Merge order

This needs the hosting .tsx build, Kong/volcano-hosting#3356 (merged as 1d1ef80). It is in staging and in kong/volcano:local-nightly, and check / localmode-e2e passed at 4ed9fc2 against that image.

Merge this only after production runs a hosting release that contains #3356 (v0.48.0 or later). Merging is releasing: release-please.yml merges the non-major release PR on its own once checks pass, and the CLI publishes about 30 minutes after this PR merges. On an older server, an uploaded index.tsx fails the build, so a project with a stray top-level .tsx would stop deploying.

Tests

Unit (make test in CLI Check): typescript_test.go covers .tsx discovery, its rank against JavaScript, Python and the other TypeScript in files and directories, the duplicate-file error, and a directory's two same-rank indexes left to the build. packager_test.go checks that page.tsx uploads as index.tsx and that a .tsx directory uploads its files under their own names. inputs_test.go checks that a .tsx file takes index.tsx in the archive and is recorded as its function's input there.
Local-mode E2E: requireLocalModeDeploysJSXFunctions in TestLocalModeE2ESmoke deploys a single .tsx file on React's automatic runtime, rendering with react-dom/server from the react and react-dom that volcano/functions/package.json installs, and a directory whose tsconfig.json sets "jsx": "react" with its own jsxFactory, then invokes both. Lanes: CLI localmode-e2e and hosting local-mode-e2e.yml.

Verification

go build ./..., go vet and go test ./internal/function pass locally. golangci-lint fmt --diff is clean on the changed packages.
I compiled both local-mode fixtures with the build's compile.mjs from Kong/volcano-hosting#3356 (at 53cc227) and ran them in Node with React 19. They returned the expected rendered values.
Deferred to CI: the full make test, golangci-lint run, and check / localmode-e2e (which waits on the hosting nightly image).
After release: a staging deploy and invoke of a react-dom/server .tsx function with the released binary.

🤖 Generated with Claude Code

Find hello.tsx and a directory's index.tsx under volcano/functions and upload them as source under their own names, index.tsx for a single file, so the Volcano build compiles their JSX. They rank with .ts, .mts and .cts: JavaScript with the same name wins, and hello.ts beside hello.tsx is a duplicate. A directory with more than one TypeScript index, such as index.ts and index.tsx, now fails the scan instead of the build.

github-actions · 2026-11-12T16:59:16Z

AI review: no P0-P2 findings

Reviewed 4ed9fc2. Run log

The CLI now finds .tsx handlers (hello.tsx and a directory's index.tsx) and uploads them as source under their own names so the Volcano build compiles their JSX. They rank after .ts, .mts and .cts, except that a directory with a JavaScript entry beside its TypeScript index ranks the way the server ranks it. Since the last review, only docs/functions.md changed: it now limits the no-change rule to .ts, .mts and .cts and dates the one exception to "CLI 0.76.0 and earlier", which resolves the earlier P3. The code's ranking matches the docs, and no defects remain. The remaining risks are release order with the hosting .tsx build, and deploy --all now treating .tsx files as functions.

Not checked: The AI review read the diff, the code, and the PR's text; it ran no tests or builds and read nothing outside this repository.

Whether Kong/volcano-hosting#3356 ranks .tsx after .ts/.mts/.cts, and ranks index.mjs beside index.tsx ahead of a sibling .ts file, as typeScriptRank and the docs say Git deploy does.
Whether the hosting build defaults .tsx to React's automatic runtime and honors a directory tsconfig's jsx/jsxFactory/jsxImportSource, as docs/functions.md says.
Whether the build deploys index.ts beside index.tsx but refuses a directory with index.ts and index.mts, as the docs say.
Whether requireLocalModeDeploysJSXFunctions passes against the published kong/volcano:local-nightly image once the hosting change lands.
Confirm Kong/volcano-hosting#3356 matches the documented source precedence, JSX defaults, and tsconfig behavior.

Human review: needed

Should deploy --all now deploy top-level .tsx files and index.tsx-only directories, which docs/functions.md said weren't functions? Such projects get new functions or failed builds.
Is it acceptable that hello/ with index.mjs or index.cjs beside index.tsx now deploys instead of hello.ts, even for -f volcano/functions/hello.ts (internal/function/typescript.go)?
Will the hosting .tsx build reach production and local-nightly before this release? Older servers refuse index.tsx, and localmode-e2e fails until then.
Should a directory containing index.mjs or index.cjs beside index.tsx replace a same-name .ts handler, even when --file explicitly names that handler (internal/function/typescript.go and internal/fun…

The AI review found no P0-P2 problems at 4ed9fc2, so the code needs no second review for bugs. Decide the points above: approve to accept them, or request changes.

Suggested reviewer: subnetmarco (most commits to internal/function scanner/packager and docs/functions.md). Any collaborator with write access other than the author can approve.

Copilot

🟡 Changes recommended

The duplicate-index scan counts files excluded by packaging’s ignore rules, incorrectly rejecting otherwise valid deployments.

1 open issue

Apply packaging ignore rules before checking duplicate directory indexes · New

What changed in this PR

Extends the CLI’s TypeScript deployment support to .tsx handlers, preserving source files for server-side JSX compilation in cloud and local mode. Release depends on hosting support for .tsx builds.

Changes:

Discover .tsx files and directory entrypoints while preserving existing runtime precedence.
Reject ambiguous TypeScript directory indexes before upload.
Update deployment help and documentation, with unit and local-mode end-to-end coverage.

File	Description
`tests/e2e/localmode/localmode_test.go`	Adds JSX deployment to the smoke test.
`tests/e2e/localmode/functions_typescript_test.go`	Tests JSX deployment and invocation with pragma and tsconfig settings.
`internal/function/typescript.go`	Recognizes `.tsx` and validates duplicate directory indexes.
`internal/function/typescript_test.go`	Covers JSX discovery, precedence, and duplicate errors.
`internal/function/scanner.go`	Carries TypeScript index metadata into source validation.
`internal/function/packager_test.go`	Verifies `.tsx` archive names and directory contents.
`internal/function/inputs_test.go`	Checks `.tsx` source input tracking.
`internal/cmd/functions/deploy.go`	Documents supported TypeScript extensions in command help.
`docs/functions.md`	Explains JSX compilation, layouts, and duplicate-index behavior.

_{Review effort: Balanced}

A directory with index.ts and index.tsx, which uploads today, failed the whole scan and blocked deploying unrelated functions, even when an ignore rule left one index out of the archive. The scan again picks the first index by name and the build refuses the pair, as it did for .ts and .mts. Pin a .tsx file's archive entry in the inputs test, and say that a single-file .tsx handler gets no tsconfig.json.

hello.ts beside hello.tsx deployed hello.ts before the CLI read .tsx, and index.ts beside index.tsx did the same in a directory. Keep it that way: .tsx ranks after .ts, .mts and .cts, files and directories alike, so such a pair deploys the .ts file instead of failing as a duplicate. The build ranks them the same way.

A directory with index.mjs beside index.tsx ranked as one with a .ts index, so it replaced a hello.ts that deployed before. It now ranks as a .tsx directory, after the .ts file.

The build compiles .tsx with React's automatic runtime unless a tsconfig sets jsx to react, so cover that default with react-dom/server instead of a pragma-only file. The docs no longer suggest pragmas.

swkeever

Guided review by agent, on Sean's delegation, at d19d774:

I checked the CLI's discovery and ranking against the server rules in Kong/volcano-hosting at 1d1ef809 (#3356, merged): internal/gitsource/scan.go, internal/gitsource/runtime.go and the public docs. Points 2 and 3 are the same decision, so I answer them together.

1. Release order vs. the hosting `.tsx` build: accept

The required checks enforce the merge order. check / localmode-e2e is a required check on main. It failed at 19:38Z because local-nightly still runs the old build, which uploads index.tsx but never compiles it: "function file not found for entry point (tried: [index.js index.mjs index.cjs])". The PR expects that failure. Hosting #3356 merged at 22:44Z, and its staging prebuild for 1d1ef809 is still running. Rerun check / localmode-e2e after a local-nightly built from 1d1ef809 or later is published, and merge only after it passes.

Release order is not enforced. The latest hosting tag, v0.48.0-rc.3, does not contain 1d1ef809. Cut the CLI release only after a production hosting release that contains 1d1ef809 is deployed. The PR's "Merge order" section already says so.

2 and 3. A top-level `.tsx` file, or a directory with only `index.tsx`, now deploys on `deploy --all`: accept

This matches Git deploy. Since #3356, the server lists .tsx in typeScriptExtensions (internal/runtimes/supported-runtimes.json; before 1d1ef809 the list was .ts, .mts, .cts). docs/public/projects/git-deploy.md:54-65 documents hello.tsx and hello/index.tsx as functions. If the CLI skipped them, one repository would deploy different functions from the CLI than from Git. The old CLI exclusion only reflected that the build did not compile .tsx yet.

No function that deploys today changes. .tsx ranks after every other entry: typescript.go:134 here, and compileRank and typeScriptRank in hosting runtime.go:97-108 and scan.go:111-124. The tests cover hello.ts beside hello.tsx, index.ts beside index.tsx, JavaScript, Python and Ruby (typescript_test.go:29-63).

A component kept as a top-level Button.tsx is the only project affected, and the documented fix is to prefix it with _ as shared code: scanner.go:82 here, and creating-functions.md "Shared code" in hosting. docs/functions.md:56-57,83-86 states the new rule. The hosting line "The CLI doesn't find .tsx files" (creating-functions.md:265) is the follow-up the PR body lists. Land it with the CLI release.

3a. `.tsx` ranking parity: revise (one case differs from the server)

Every case matches the server except a directory whose index.tsx has a JavaScript entry beside it.

Server: it ranks a directory by its entry. eligibleEntries (scan.go:431-443) keeps index.mjs over index.tsx, and compileRank("…/index.mjs") == 0, so typeScriptRank returns 0 (scan.go:115-119).
CLI: it ranks the directory by the .tsx index that found it. typeScriptDirectoryEntry returns viaJSX=true on the JavaScript-sibling path (typescript.go:73-76), so typeScriptRank returns 3 (typescript.go:128-146, and the comment at typescript.go:132 says so on purpose).

Results:

page.ts + page/index.mjs + page/index.tsx: Git deploy deploys page/ (index.mjs, rank 0 < 2). The CLI deploys page.ts (2 < 3). typescript_test.go:37 asserts the CLI result, which differs from the server.
page.ts + page.mts + page/index.mjs + page/index.tsx: Git deploy deploys page/. The CLI stops with "two TypeScript files".

The .ts version of the same layout agrees: page.ts + page/index.mjs + page/index.ts puts page/ at rank 1 in the CLI, and that test passes (typescript_test.go:38). The PR text and hosting scan.go:114 both say the CLI ranks the same way. Without a fix, the same repository deploys a different handler from the CLI than from Git.

Required change: on the JavaScript-sibling path in typeScriptDirectoryEntry, return viaJSX=false, so the directory ranks like the .ts case (return *c.typeScript, js, false, true at typescript.go:76). Change typescript_test.go:37 to expect page/index.mjs, and update the comments at typescript.go:57-61 and typescript.go:128-133. I applied exactly this in a scratch worktree at d19d774 and ran go test ./internal/function -count=1, which passed. The only test that changed was line 37.

Not in scope, and older than this PR (from #428): the CLI never finds JavaScript entries other than the runtime entrypoint, such as foo/index.mjs alone or foo/util.js. So foo.mjs beside foo/index.ts + foo/util.js deploys foo.mjs from the CLI, while Git deploy refuses it as a duplicate. Fix that separately if it matters.

Not checked

Local-mode E2E against a local-nightly that contains 1d1ef809. The image is not published yet, so point 1 depends on that rerun.
The JSX runtime defaults and tsconfig jsx/jsxFactory/jsxImportSource handling in the hosting build. Those are server behavior, which the hosting PR's tests cover (infrastructure/codebuild/typescript/jsx.test.mjs). The CLI only uploads .tsx as source.

Verdict: REVISE. Rank a directory whose index.tsx has an index.js, index.mjs or index.cjs beside it like a .ts directory (viaJSX=false on that path, typescript.go:76), and change typescript_test.go:37 to expect page/index.mjs. Points 1 and 2 are accepted as they are.

Generated by the guided-review skill. Install it with one command:

sh -c 'd=$(mktemp -d) && gh repo clone Kong/volcano-conventions "$d" -- --depth 1 -q && make -C "$d" install-skills guided-review pr-review explain-pr; s=$?; rm -rf "$d"; exit $s'

Then ask your coding agent for a guided review of a PR or issue URL. Run the command again to update.

A directory whose index.mjs sits beside index.tsx deploys index.mjs, and the server ranks it ahead of a TypeScript file, as it ranks any JavaScript entry. The CLI ranked it as a .tsx directory, so page.ts beside page/ with index.mjs and index.tsx deployed page.ts from the CLI but page/ from Git. Rank it as a directory with a .ts index, as index.mjs beside index.ts already is.

swkeever

Guided review by agent, on Sean's delegation, at 4ed9fc2:

I checked the CLI's discovery and ranking against the server at Kong/volcano-hosting origin/main (4ae5154d, which contains #3356 / 1d1ef809): internal/gitsource/scan.go, internal/gitsource/runtime.go, infrastructure/codebuild/function-compile.sh, and docs/public/functions/creating-functions.md. At this head, go test ./internal/function -run 'TestScanSources|TestPackageSource|TestInputPaths' -count=1 passes. check / localmode-e2e passed at this head against the new nightly (run 43326153539, success). The AI review's points 2 and 4 ask the same question, so I answer them together.

Earlier guided review (at `d19d774`): addressed

That review asked for one change: on the JavaScript-sibling path, return viaJSX=false, so page/ with index.mjs beside index.tsx ranks like a .ts directory. This head makes that change: internal/function/typescript.go:77 returns *c.typeScript, js, false, true. typescript_test.go:37 now expects page/index.mjs over page.ts, and the comments at typescript.go:57-64 and :129-133 describe the new behavior. I checked this against the server myself. typeScriptRank (scan.go:115-125) takes compileRank of the directory's entry. compileRank("index.mjs") is 0 (runtime.go:101-110), so the server deploys page/ too. With jsxCompileRank = 2 (runtime.go:95), the server's order is: non-TypeScript 0, .ts directory 1, .ts file 2, .tsx directory 3, .tsx file 4. The CLI's typeScriptRank (typescript.go:134-146) gives the same order.

There is one remaining difference, and it is older than this PR (it dates from #428). The CLI ranks a directory whose JavaScript entry sits beside a TypeScript index at 1, where the server ranks it at 0. Against a rank-0 rival such as page.js, Git deploy refuses the duplicate, but the CLI deploys page.js. The .ts case already behaves this way, so this PR doesn't need to fix it.

1. `deploy --all` now deploys a top-level `.tsx` file and an `index.tsx`-only directory: accept

This matches Git deploy, which already treats both as functions: hosting docs/public/projects/git-deploy.md:54-65, and function-compile.sh:1851-1865 compiles .tsx only when no .ts/.mts/.cts handler exists. If the CLI skipped them, one repository would deploy different functions from the CLI than from Git.

No function that deploys today changes. .tsx is last in typeScriptExtensions (typescript.go:27) and ranks last in typeScriptRank (typescript.go:139-140). typescript_test.go:29-41,61-63 covers .tsx beside JavaScript, Python, .ts/.mts/.cts files, and indexes. Only one kind of project is affected: one that keeps a non-handler component as a top-level volcano/functions/Button.tsx, which will now fail its build. The fix is the existing _ shared-code prefix (scanner.go:82). docs/functions.md:56-57,83-86 states the rule. Hosting creating-functions.md:265 ("The CLI doesn't find .tsx files") becomes wrong once this releases. The PR body lists that follow-up, and it should land with the CLI release.

2 and 4. `hello/` with `index.mjs`/`index.cjs` beside `index.tsx` replaces `hello.ts`, even for `-f volcano/functions/hello.ts`: accept

This is the server's result: dropShadowedTypeScript (scan.go:101-109) keeps hello/ at rank 0 over hello.ts at rank 2. Before this PR, the CLI didn't find that directory at all. That made it the odd one out, not the server.

-f resolves to the winning source of the function. FindTarget (scanner.go:126-148) matches by path among the scanned sources. The shadowed hello.ts isn't one of them, so the lookup falls back to the name hello. A .ts directory has worked the same way since #428 (typescript_test.go:38). The alternative, deploying the shadowed file because -f named it, would make -f and deploy --all deploy different handlers under one function name. The layout is rare, since it has two handlers for one name. The deploy output names the detected entry, and docs/functions.md:87-90 documents it as the one change from CLI 0.76.0.

However, the PR body's "User-facing behavior change" section says "Nothing else changes: every function that deploys today deploys the same file". That contradicts docs/functions.md:87-90 and this exact case. The body must name this layout.

3. Release order vs. the hosting `.tsx` build: the order is acceptable only with a hold, and the PR body is wrong about when to hold

Staging and local-nightly: done. Staging-pipeline run 43332243037 ran at 484e4d3f, which contains 1d1ef809 (compare: ahead 4, behind 0). check / localmode-e2e passed at this head.
Production: not yet. The latest non-prerelease hosting release is v0.47.0, created 11-12 11:04Z, which predates the #3356 merge. Only the prereleases v0.48.0-rc.5 and rc.6 contain 1d1ef809.
The release follows the merge automatically: .github/workflows/release-please.yml runs on every push to main. Its "Merge non-major release PRs" step auto-merges the release: X.Y.Z PR once its checks pass, in about 30 minutes, and publish-cli.yml publishes the CLI. "Merge, but release later" is not possible here.

A production server without #3356 can't build an uploaded index.tsx. The build tries only index.js/.mjs/.cjs, so users who follow the new docs get a failed build with a misleading error. The required condition: merge only after production runs a hosting release that contains 1d1ef809 (v0.48.0 or later). The PR's "Merge order" section says local-nightly gates the merge and production gates only the release. Under auto-release, that sequence ships a CLI ahead of production.

Not checked

JSX defaults and tsconfig jsx/jsxFactory/jsxImportSource handling in the build. That is server behavior, and the passing local-mode cell (functions_typescript_test.go:441-502, which covers the automatic runtime and a classic jsxFactory) exercises it.
A cloud deploy against staging with a released binary. The PR lists it for after release.

Verdict: REVISE

What must change before merge:

PR body, "User-facing behavior change": replace "Nothing else changes" with the one changed layout. hello/ with index.mjs or index.cjs beside index.tsx now deploys instead of hello.ts, including for -f volcano/functions/hello.ts, as Git deploy does.
PR body, "Merge order": say that merging releases the CLI automatically (release-please.yml auto-merges non-major release PRs), so the PR merges only after production runs a hosting release containing 1d1ef809 (v0.48.0 or later).
Hold the merge until that production release is deployed. No code change is needed. Points 1, 2 and 4 are accepted as the code stands.

Generated by the guided-review skill. Install it with one command:

sh -c 'd=$(mktemp -d) && gh repo clone Kong/volcano-conventions "$d" -- --depth 1 -q && make -C "$d" install-skills guided-review pr-review explain-pr; s=$?; rm -rf "$d"; exit $s'

Then ask your coding agent for a guided review of a PR or issue URL. Run the command again to update.

swkeever requested a review from a team as a code owner November 12, 2026 16:55

Copilot AI balanced review requested due to automatic review settings November 12, 2026 16:55

Copilot started reviewing on behalf of swkeever November 12, 2026 16:56

View session

github-actions Bot requested a review from subnetmarco November 12, 2026 16:59