Skip to content

fix: allow false for resolver request/response mapping templates (#598)#714

Merged
sid88in merged 1 commit into
masterfrom
fix/598-resolver-mapping-template
Jun 1, 2026
Merged

fix: allow false for resolver request/response mapping templates (#598)#714
sid88in merged 1 commit into
masterfrom
fix/598-resolver-mapping-template

Conversation

@sid88in
Copy link
Copy Markdown
Owner

@sid88in sid88in commented May 31, 2026

Fix resolver request/response validation to allow false

Fixes #598.

Problem

The resolver config type allows the mapping templates to be false:

export type BaseResolverConfig = {
  // ...
  request?: string | false;
  response?: string | false;
};

…but the validation schema only accepts strings, so a config like request: false is rejected before it ever reaches the resolver compiler:

at appSync/.../request: must be string

This is a type-vs-validation mismatch: the type (and the compiler) support false, but validation does not.

Why false is a real feature

The resolver compiler already handles false correctly:

  • isVTLResolver = 'request' in this.config || 'response' in this.config — setting request: false still puts the resolver on the VTL path (rather than being treated as a JS resolver), and
  • resolveMappingTemplate() does const templateName = this.config[type]; if (templateName) { … } — with false, that's falsy, so the corresponding RequestMappingTemplate / ResponseMappingTemplate simply isn't emitted.

So false is a deliberate way to declare a VTL resolver that omits a request and/or response mapping template. Only the schema was out of sync.

Change

  • Adds a mappingTemplate schema definition: oneOf: [{ type: 'string' }, { const: false }], with a clear error message.
  • Points resolverConfig.request / resolverConfig.response at it.
  • No type changes were needed — the types already declared string | false; this brings validation in line with them.

The validation error message changes from must be string to must be a string (path to the template) or false to omit the mapping template, which the existing negative-case snapshot reflects.

Out of scope (intentional)

Pipeline functions (PipelineFunctionConfig) keep request/response as string-only. There the type and the schema already agree (both string-only), so there's no mismatch to fix; adding false there would be a separate feature change (type + schema) rather than a bug fix.

Tests

  • Added a valid case: a UNIT resolver with request: false / response: false, and one with request: false + a response template path.
  • Added an invalid case: request: true / response: true is rejected (guards against widening to any boolean).
  • Existing negative case (request: 123) still fails, with the updated message.
  • Full suite green: npm test (333), npm run test:e2e (84), npm run lint (0 errors), npm run build.

Summary by CodeRabbit

  • New Features

    • Resolver configuration validation now supports optional mapping templates, allowing them to be disabled with false in addition to specifying template paths.
  • Tests

    • Expanded validation test suite with cases for resolver configurations with disabled mapping templates.

The BaseResolverConfig type allows request/response to be string | false
and the resolver compiler already handles false (it keeps the VTL path
and omits the mapping template), but the validation schema only accepted
strings, rejecting valid configs. Adds a mappingTemplate schema
definition (string or false) and points resolverConfig.request/response
at it. Pipeline functions stay string-only (type and schema already
agree). Adds valid (false) and invalid (true) test cases.

Fixes #598
@sid88in sid88in requested a review from AlexHladin May 31, 2026 20:51
@sid88in sid88in self-assigned this May 31, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 31, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ceb2d0a6-5c09-4e5a-9466-3b9983d59df4

📥 Commits

Reviewing files that changed from the base of the PR and between 6bbd830 and 0fd2d08.

⛔ Files ignored due to path filters (1)
  • src/__tests__/validation/__snapshots__/resolvers.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (2)
  • src/__tests__/validation/resolvers.test.ts
  • src/validation.ts

📝 Walkthrough

Walkthrough

This PR extends GraphQL AppSync schema validation to allow resolver mapping templates to be explicitly disabled via false values. The mappingTemplate definition is updated to accept either a string path or false, and resolver request/response fields are modified to reference this definition. Test cases validate both the successful disabling of mapping templates and rejection of invalid boolean true values.

Changes

Mapping Template Validation

Layer / File(s) Summary
Schema validation contract for mapping templates
src/validation.ts
mappingTemplate schema updated to allow string or { const: false }, with resolver request and response fields changed to reference this definition instead of plain string validation.
Test cases for disabled and invalid mapping templates
src/__tests__/validation/resolvers.test.ts
Added valid case for resolvers with request: false and response: false, and invalid case asserting rejection when mapping templates are set to boolean true instead of string or false.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A schema opens wide with false,
No template needed, mapping halls,
Resolvers dance in validation's light,
Request and response set just right! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: allowing false as a valid value for resolver request/response mapping templates alongside strings.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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/598-resolver-mapping-template

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

@sid88in sid88in merged commit 978c4cb into master Jun 1, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validation does not match types ( request and response in resolverConfig )

2 participants