Project typescript support#170
Draft
stevendborrelli wants to merge 7 commits into
Draft
Conversation
Signed-off-by: Steven Borrelli <steve@borrelli.org>
Signed-off-by: Steven Borrelli <steve@borrelli.org>
stevendborrelli
requested review from
a team,
jcogilvie and
tampakrap
as code owners
Contributor
📝 WalkthroughWalkthroughAdds end-to-end TypeScript support to the Crossplane CLI: a new ChangesTypeScript Support
Sequence DiagramsequenceDiagram
rect rgba(70, 130, 180, 0.5)
Note over Builder,SchemaManager: Schema generation
Builder->>DependencyManager: CollectSources(ctx, ch)
DependencyManager-->>Builder: []Source (xpkg resolved + versioned)
Builder->>SchemaManager: GenerateFromMultipleSources(allSources)
SchemaManager->>typescriptGenerator: GenerateFromCRD(mergedFS)
typescriptGenerator->>SchemaRunner: run crd-generate + tsc in node container
SchemaRunner-->>typescriptGenerator: compiled models/
typescriptGenerator-->>SchemaManager: output FS
SchemaManager->>SchemaManager: copy to schema repo, postProcess, update versions
end
rect rgba(60, 179, 113, 0.5)
Note over Builder,typescriptBuilder: Function build
Builder->>typescriptBuilder: Build(ctx, functionFS)
typescriptBuilder->>SchemaRunner: buildFunction (npm install + build in node container)
SchemaRunner-->>typescriptBuilder: /function tar
typescriptBuilder->>RuntimeImage: fetch distroless nodejs base per arch
typescriptBuilder->>RuntimeImage: append function layer + configureTypescriptImage
RuntimeImage-->>Builder: arch-specific OCI images
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
internal/project/functions/typescript.go (1)
59-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTiny doc nit: the comment says
npm cibut the build runsnpm install.The struct doc says we build via
npm ci, but both build scripts usenpm install(and the const comment even explains why). Mind tweaking the comment so it matches the actual behavior? Totally optional, just to avoid confusing future readers. 🙂📝 Suggested wording
-// A TypeScript embedded function is a full function-sdk-typescript project -// (package.json + src/). We build it by running npm ci and npm run build -// (which invokes tsgo) in a Node.js build container, then copy the dist/ -// and node_modules/ onto a distroless Node.js base. +// A TypeScript embedded function is a full function-sdk-typescript project +// (package.json + src/). We build it by running npm install and npm run build +// (which invokes tsgo) in a Node.js build container, then copy the dist/ +// and node_modules/ onto a distroless Node.js base.🤖 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 `@internal/project/functions/typescript.go` around lines 59 - 64, The TypeScript builder doc comment is out of sync with the actual install step, since `typescriptBuilder` uses `npm install` rather than `npm ci`. Update the comment on `typescriptBuilder` (and any nearby related comment if needed) so it accurately describes the build flow: running `npm install` and `npm run build` in the Node.js build container before copying `dist/` and `node_modules/`.
🤖 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 `@cmd/crossplane/function/generate.go`:
- Around line 417-423: The `generate.go` schema probe is swallowing the
`afero.DirExists` failure on `schemasFS` under `typescript`, which can hide real
filesystem errors. Update the `hasSchemas` check in the schema-loading path to
capture and return the `DirExists` error instead of defaulting to “no schemas,”
and keep the subsequent `afero.ReadDir` handling in the same flow so failures
are propagated clearly from this code path.
In `@internal/project/functions/typescript.go`:
- Line 168: The `afero.DirExists` call is ignoring its error, so transient
filesystem failures can be mistaken for “no schemas” and silently skip the
schemas layer. Update the code around the `hasTSSchemas` check to handle and
propagate the `DirExists` error the same way the sibling `match` logic handles
`DirExists`/`Exists`, using the surrounding function that builds the TypeScript
schema detection flow.
- Around line 49-56: The package-level typescriptBuildScript constant is dead
code because buildFunction uses its own inline buildScript instead. Either
remove typescriptBuildScript from internal/project/functions/typescript.go if it
is no longer needed, or update buildFunction to reference typescriptBuildScript
so there is a single source of truth for the TypeScript build pipeline.
- Around line 166-210: The TypeScript build path still hardcodes the schemas
container location in the buildScript inside typescript.go, so it can miss
dependencies when c.SchemasPath changes. Update the logic in the function that
prepares schemasTar and constructs the container command to derive the
in-container schemas path from c.SchemasPath (using the same tsSchemasRel/base
path used for FSToTar and StartWithCopyFiles) instead of checking
/schemas/typescript, so the npm install branch follows the configured schemas
root.
- Around line 47-48: Update the TypeScript runtime base image constant used by
baseImageForArch from gcr.io/distroless/nodejs24-debian12 to the published
gcr.io/distroless/nodejs24-debian13 tag. Keep the change confined to the
typescriptRuntimeImage symbol in internal/project/functions/typescript.go so any
runtime image resolution uses the correct Node 24 distroless base.
In `@internal/schemas/generator/interface.go`:
- Line 46: The default AllLanguages() registry currently includes
typescriptGenerator, which makes TypeScript generation run for projects that did
not explicitly opt in. Update the schema language selection logic so TypeScript
is only added when schemas.languages explicitly includes "typescript" or when a
dedicated feature flag enables it, and keep the default generator set unchanged
for existing builds. Use the AllLanguages() function and &typescriptGenerator{}
as the main points to adjust.
In `@internal/schemas/generator/typescript.go`:
- Around line 219-227: stagedCRDPath currently flattens nested paths by
replacing "/" with "_" in the staged filename, which can make distinct source
paths collide and overwrite each other. Update stagedCRDPath to preserve path
uniqueness when generating the staged CRD name, using a collision-resistant
encoding of the original sourcePath (including directories) while still applying
the suffix/extension logic, and ensure any caller relying on staged TypeScript
CRD paths uses the updated naming consistently.
- Line 43: The TypeScript generator setup is currently pulling dependencies at
runtime with floating versions, which makes schema generation non-reproducible.
Update the generator flow around typescriptImage and the TypeScript generator
invocation to use pinned dependencies via a lockfile with npm ci, or bake
crd-generate and its dependencies into the image so the generation environment
is deterministic. Make sure any npm install usage and ^ version ranges are
removed or replaced with exact pinned versions.
In `@internal/schemas/manager/manager.go`:
- Around line 288-314: The merged source prefix in manager.go can collide
because sanitizeSourceID() may produce the same directory for different source
IDs, causing resources to overwrite during CopyFilesBetweenFs. Update the prefix
generation in the merge loop (and the matching logic in the other affected
block) to include a collision-resistant suffix such as the source index or a
stable hash derived from src.ID(), while keeping the existing source ID context.
Use the same prefix strategy wherever mergedFS/prefixedFS is built so each
source gets a unique namespace.
---
Nitpick comments:
In `@internal/project/functions/typescript.go`:
- Around line 59-64: The TypeScript builder doc comment is out of sync with the
actual install step, since `typescriptBuilder` uses `npm install` rather than
`npm ci`. Update the comment on `typescriptBuilder` (and any nearby related
comment if needed) so it accurately describes the build flow: running `npm
install` and `npm run build` in the Node.js build container before copying
`dist/` and `node_modules/`.
🪄 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: 4c74d371-9f20-491a-96b8-a83f8b3f9eb8
⛔ Files ignored due to path filters (5)
.github/renovate.json5is excluded by none and included by nonecmd/crossplane/function/templates/typescript/package.json.tmplis excluded by none and included by nonecmd/crossplane/function/templates/typescript/src/function.tsis excluded by none and included by nonecmd/crossplane/function/templates/typescript/src/main.tsis excluded by none and included by nonecmd/crossplane/function/templates/typescript/tsconfig.jsonis excluded by none and included by none
📒 Files selected for processing (11)
apis/dev/v1alpha1/project_types.gocmd/crossplane/function/generate.gocmd/crossplane/function/help/generate.mdcmd/crossplane/function/templates/typescript/README.mdinternal/dependency/manager.gointernal/project/build.gointernal/project/functions/build.gointernal/project/functions/typescript.gointernal/schemas/generator/interface.gointernal/schemas/generator/typescript.gointernal/schemas/manager/manager.go
Comment on lines
+47
to
+48
| // typescriptRuntimeImage is the distroless base used at runtime. | ||
| typescriptRuntimeImage = "gcr.io/distroless/nodejs24-debian12" |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
gcr.io/distroless/nodejs24 available debian tag latest
💡 Result:
The Distroless Node.js 24 image is available under the repository name gcr.io/distroless/nodejs24-debian13 [1][2]. You can use the tag latest with this image (e.g., gcr.io/distroless/nodejs24-debian13:latest) [2][3]. Other available tags for this image include nonroot, debug, and debug-nonroot [2][3]. These images are maintained by the GoogleContainerTools project and are updated regularly to include the latest stable versions of Node.js [1][4].
Citations:
- 1: https://github.com/GoogleContainerTools/distroless/blob/main/nodejs/README.md
- 2: https://github.com/GoogleContainerTools/distroless/blob/main/README.md
- 3: https://raw.githubusercontent.com/GoogleContainerTools/distroless/main/README.md
- 4: Bumping node archives to latest stable versions GoogleContainerTools/distroless#2046
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files internal/project/functions/typescript.go
wc -l internal/project/functions/typescript.go
sed -n '1,260p' internal/project/functions/typescript.go
rg -n "typescriptRuntimeImage|nodejs24-debian12|nodejs24-debian13|baseImageForArch" internal/project/functions -SRepository: crossplane/cli
Length of output: 9719
Switch the TypeScript runtime image to nodejs24-debian13
gcr.io/distroless/nodejs24-debian12 doesn’t appear to be published; the Node 24 distroless runtime is gcr.io/distroless/nodejs24-debian13, so this pull will fail when baseImageForArch tries to fetch the base image. Could this tag be updated?
🔧 Proposed change
- typescriptRuntimeImage = "gcr.io/distroless/nodejs24-debian12"
+ typescriptRuntimeImage = "gcr.io/distroless/nodejs24-debian13"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // typescriptRuntimeImage is the distroless base used at runtime. | |
| typescriptRuntimeImage = "gcr.io/distroless/nodejs24-debian12" | |
| // typescriptRuntimeImage is the distroless base used at runtime. | |
| typescriptRuntimeImage = "gcr.io/distroless/nodejs24-debian13" |
🤖 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 `@internal/project/functions/typescript.go` around lines 47 - 48, Update the
TypeScript runtime base image constant used by baseImageForArch from
gcr.io/distroless/nodejs24-debian12 to the published
gcr.io/distroless/nodejs24-debian13 tag. Keep the change confined to the
typescriptRuntimeImage symbol in internal/project/functions/typescript.go so any
runtime image resolution uses the correct Node 24 distroless base.
| // the relative path in package.json (e.g., "file:../../schemas/typescript"). | ||
| tsSchemasRel := path.Join(c.SchemasPath, "typescript") | ||
| tsSchemasFS := afero.NewBasePathFs(c.ProjectFS, tsSchemasRel) | ||
| hasTSSchemas, _ := afero.DirExists(tsSchemasFS, ".") |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Swallowed error on afero.DirExists.
The sibling match method checks the DirExists/Exists errors, but here the error is dropped. A transient FS error would be misread as "no schemas," silently skipping the schemas layer rather than failing loudly. Want to propagate it for consistency?
🛡️ Suggested change
- hasTSSchemas, _ := afero.DirExists(tsSchemasFS, ".")
+ hasTSSchemas, err := afero.DirExists(tsSchemasFS, ".")
+ if err != nil {
+ return nil, errors.Wrap(err, "failed to check for typescript schemas")
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| hasTSSchemas, _ := afero.DirExists(tsSchemasFS, ".") | |
| hasTSSchemas, err := afero.DirExists(tsSchemasFS, ".") | |
| if err != nil { | |
| return nil, errors.Wrap(err, "failed to check for typescript schemas") | |
| } |
🤖 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 `@internal/project/functions/typescript.go` at line 168, The `afero.DirExists`
call is ignoring its error, so transient filesystem failures can be mistaken for
“no schemas” and silently skip the schemas layer. Update the code around the
`hasTSSchemas` check to handle and propagate the `DirExists` error the same way
the sibling `match` logic handles `DirExists`/`Exists`, using the surrounding
function that builds the TypeScript schema detection flow.
Signed-off-by: Steven Borrelli <steve@borrelli.org>
Signed-off-by: Steven Borrelli <steve@borrelli.org>
Signed-off-by: Steven Borrelli <steve@borrelli.org>
Signed-off-by: Steven Borrelli <steve@borrelli.org>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Add TypeScript support for composition functions
Fixes #169
Testing Typescript A guide to testing this is in docs/typescript-testing-guide.md , available via the PR link at https://github.com/crossplane/cli/pull/170/files#diff-7211df7c8b1305a9f0fec2a983e06ac51b4ed66dda9f91ed158250904c9c7e6d
Description
This PR adds first-class TypeScript support to the Crossplane CLI, enabling developers to write composition functions in TypeScript with full type safety. An example project using this branch is https://github.com/stevendborrelli/configuration-aws-network-ts-xp-cli.
Changes
TypeScript Function Builder (
internal/project/functions/typescript.go)package.json+src/directorynode:24-slim)npm installandnpm run build(typically invokingtsgo)gcr.io/distroless/nodejs24-debian12)crossplane-modelsschema package as afile:dependencyTypeScript Schema Generation (
internal/schemas/generator/typescript.go)@kubernetes-models/crd-generate@kubernetes-models/basecrossplane-modelsnpm package that functions can importcrossplane-models/ec2.aws.upbound.io/v1beta1Merged Schema Generation (
internal/schemas/manager/manager.go,internal/project/build.go)GenerateFromMultipleSources()to generate schemas from all CRD sources in a single passindex.jswith proper cross-referencesFunction Template (
cmd/crossplane/function/generate.go)crossplane function init --language=typescriptpackage.jsonwith SDK dependenciestsconfig.json)Example Usage
Generated Package Structure
Dependencies
This feature builds on existing ecosystem work:
@crossplane-org/function-sdk-typescript- Official TypeScript SDK for composition functions@kubernetes-models/crd-generate- Generates TypeScript classes from CRDs with constructors, interfaces, and runtime validationtsgo- Fast TypeScript compiler for build pipelinesTesting
crossplane project buildbuilds TypeScript functionscrossplane function init --language=typescriptcreates a working templateBreaking Changes
None. This is additive functionality.
Checklist
I have:
./nix.sh flake checkto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.