feat: support TypeScript files in ${file()} variable resolver#13590
Conversation
Extend the `${file(...)}` resolver to load `.ts`, `.mts`, and `.cts`
modules in addition to the existing `.js`/`.cjs`/`.mjs` support.
TypeScript modules are compiled on the fly via tsx — the same loader
the framework already uses for `serverless.ts` — so no separate build
step is needed.
The JS and TS code paths share a single export-shape handler, so all
supported shapes (default object, async default function, named export,
named export function with property selector, injected
`resolveVariable` / `resolveConfigurationProperty` callbacks) behave
identically across JavaScript and TypeScript sources. tsx is loaded
lazily so cold-start is unchanged when no TS reference is present.
Errors are surfaced as either "Cannot load TS module" (transpile or
import failure) or "Cannot execute TS module" (runtime throw inside
the module body) for clearer diagnostics.
📝 WalkthroughWalkthroughAdds runtime TypeScript loading (via tsx) and unified JS/TS module resolution for variables: resolveLoadedModule normalizes exports, invokes callable exports with resolver context, supports propertyPath selectors, includes tests for routing/interop, and updates documentation. ChangesTypeScript and JavaScript Module Resolution
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/sf/guides/variables/file.md (1)
89-96: ⚡ Quick winConsider adding context for the injected function parameters.
Line 89 mentions that functions receive
{ options, resolveVariable, resolveConfigurationProperty }but doesn't explain what these are or when they'd be useful. The example function doesn't use any of these parameters, which leaves their purpose unclear.Consider either:
- Adding a brief explanation of these parameters (e.g., "
resolveVariablelets you resolve other serverless variables from within your function")- Or providing a second example that demonstrates using one of them
This would help users understand when and why they might need these capabilities.
🤖 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 `@docs/sf/guides/variables/file.md` around lines 89 - 96, The doc mentions that exported functions receive `{ options, resolveVariable, resolveConfigurationProperty }` but doesn't explain their purpose; update the text around the `getSecrets` example to briefly describe each parameter (e.g., `options` is the CLI/deployment options, `resolveVariable` lets you synchronously resolve other serverless variables from inside your function, and `resolveConfigurationProperty` lets you resolve typed configuration properties) and either modify the `getSecrets` example or add a second short example showing use of one of them (for instance call `resolveVariable('env:API_KEY')` inside `getSecrets`) so readers can see a practical use-case.
🤖 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.
Nitpick comments:
In `@docs/sf/guides/variables/file.md`:
- Around line 89-96: The doc mentions that exported functions receive `{
options, resolveVariable, resolveConfigurationProperty }` but doesn't explain
their purpose; update the text around the `getSecrets` example to briefly
describe each parameter (e.g., `options` is the CLI/deployment options,
`resolveVariable` lets you synchronously resolve other serverless variables from
inside your function, and `resolveConfigurationProperty` lets you resolve typed
configuration properties) and either modify the `getSecrets` example or add a
second short example showing use of one of them (for instance call
`resolveVariable('env:API_KEY')` inside `getSecrets`) so readers can see a
practical use-case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14ab977a-5abb-4463-872b-fd1cab5bd236
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
docs/sf/guides/variables/file.mdpackages/sf-core/package.jsonpackages/sf-core/src/lib/resolvers/providers/file/file.jspackages/sf-core/tests/unit/resolvers/file-ts.test.js
tsx's tsImport compiles a `.ts` file with `export default X` to CJS and
returns a Module Namespace shaped as:
{ default: { __esModule: true, default: X, ...named } }
The inner namespace has a null prototype. Returning it verbatim from the
file resolver crashed the resolver manager with
`TypeError: Cannot convert object to primitive value` whenever the
result was chained through another placeholder (e.g.
`${self:custom.secrets.apiKey}` reading from
`secrets: ${file(./scripts/secrets.ts)}`) — String.prototype.replace
inside #updateNodePlaceholders cannot coerce a null-prototype Module
Namespace to a primitive.
Native dynamic `import()` of a `.cjs` or `.js` file collapses this layer
for us via the documented `__esModule` interop convention; tsx does
not. Peel one layer in resolveLoadedModule when `module.default.__esModule`
is truthy so the rest of the function operates on the user's intended
module shape regardless of loader. The check is loader-agnostic — Babel,
TypeScript, esbuild, tsx, and any future loader following the convention
are all normalized the same way. Falsy `__esModule` (or a non-object
default, like `export default 42`) is a no-op, preserving existing
behavior for the JS code path and for `.ts` modules whose default is a
plain value.
Adds 7 unit tests covering:
- selector-less default-only TS module (the failing scenario today)
- the result is a plain Object.prototype object AND survives the exact
String.replace call that crashed the manager (locks in the regression)
- property selector against a named export under the __esModule wrap
- property selector against the default object under the __esModule wrap
- async default-export function under the wrap, invoked with context
- no unwrap when __esModule marker is absent
- no unwrap when default is a non-object
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2b2ace5. Configure here.
Summary
Extend the
${file(...)}variable resolver to load TypeScript modules (.ts,.mts,.cts) in addition to the existing JavaScript support (.js,.cjs,.mjs). TypeScript files are compiled on the fly via tsx — the same loader the Framework already uses forserverless.ts— so no separate build step is required.Before
After
Both forms work. From
serverless.ymlorserverless.ts:Details
.ts/.mts/.ctscases to the file resolver.#propertyselector, and injectedresolveVariable/resolveConfigurationPropertycallbacks all behave the same across JS and TS sources.tsxis loaded lazily inside the TS branch so cold-start is unchanged when no TS reference is present.Cannot load TS module "..."for transpile / import failures, andCannot execute TS module "..."for runtime throws inside the module body.tsxas an explicit dependency of@serverlessinc/sf-core(previously resolved transitively).Behavior note
The shared module-shape handler now unwraps the standard CJS↔ESM interop layer (the
__esModule: truemarker) once before reading exports. Native dynamicimport()of a.cjsfile already does this collapse implicitly;tsx'stsImportdoes not, hence the explicit unwrap. As a side effect this also applies to the JS code path:.cjs/.jsfiles whosemodule.exportscarry an__esModule: truemarker (i.e. anything emitted by Babel / TypeScript / esbuild in CJS mode) now return the user's intended default export from${file(./compiled.js)}and from${file(./compiled.js):default}— instead of the transpiler wrapper object that was previously returned. This is a correctness fix; the prior wrapper was an interop leak, not an intended contract.__esModulemarker, default exports that are not objects (e.g.export default 42), and all hand-authored JS / YAML / JSON / raw-text references are unaffected.Test plan
npm run prettier— cleannpm run lint— cleannpm run test:unitinpackages/sf-core— 386 tests pass (23 pre-existing JS resolver tests unchanged + 11 new TS resolver tests)npm run buildinpackages/sf-core— bundle produces successfullysls printagainst a TS-only repro covering: default-export object, property selector, async named-export function with env-var read, named export from a.tsfile usingas constCannot load TS modulewith the underlying esbuild transform error; TS file whose function throws surfacesCannot execute TS modulewith the original error message