Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix/domains multiple api types lazy evaluation#13310

Closed
cstea wants to merge 2 commits intoserverless:mainserverless/serverless:mainfrom
cstea:fix/domains-multiple-api-types-lazy-evaluationcstea/serverless:fix/domains-multiple-api-types-lazy-evaluationCopy head branch name to clipboard
Closed

Fix/domains multiple api types lazy evaluation#13310
cstea wants to merge 2 commits intoserverless:mainserverless/serverless:mainfrom
cstea:fix/domains-multiple-api-types-lazy-evaluationcstea/serverless:fix/domains-multiple-api-types-lazy-evaluationCopy head branch name to clipboard

Conversation

@cstea
Copy link

@cstea cstea commented Feb 1, 2026

Summary

  • Fix: Uses lazy evaluation for getDefaultApiType() so it's only called when a domain actually needs a default API type
  • When all domains have explicit apiType, no API type detection is performed, avoiding the erroneous "Multiple API types
    detected" error
  • The default API type is computed once and cached when needed

Problem

Previously, getDefaultApiType() was called unconditionally during initializeVariables(). When multiple API types
(HTTP, REST, WebSocket) were detected in the CloudFormation template, it threw an error:

Multiple API types detected in CloudFormation template: HTTP, WEBSOCKET.
Please explicitly specify the apiType for each domain configuration.

This happened even when all domain configurations already had explicit apiType specified, making it impossible to use
multi-domain configurations with different API types as documented.

Solution

Implement lazy evaluation: getDefaultApiType() is now only called when a domain configuration actually needs a default
(when apiType is not explicitly specified). The result is cached to avoid redundant computations.

Test plan

  • Added 29 unit tests covering getDefaultApiType(), detectApiTypesFromTemplate(), and initializeVariables()
  • Tests verify lazy evaluation behavior (getDefaultApiType not called when not needed)
  • All existing tests pass
  • npm run lint passes
  • npm run prettier passes

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes & Improvements

    • Enhanced domain configuration with optimized API type detection and improved deferred computation handling.
    • Better fallback logic for domains without explicit API type specifications.
  • Tests

    • Added comprehensive test coverage for domain configuration, API type detection, and multi-API type scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

cstea and others added 2 commits January 31, 2026 23:17
Previously, getDefaultApiType() was called unconditionally during
initializeVariables(), throwing an error when multiple API types
(HTTP, REST, WebSocket) were detected in the CloudFormation template,
even when all domain configurations had explicit apiType specified.

This change implements lazy evaluation: getDefaultApiType() is now
only called when a domain configuration actually needs a default
(when apiType is not explicitly specified). The result is cached to
avoid redundant computations.

This allows users to configure multiple domains with different API
types (e.g., HTTP and WebSocket) as documented, as long as each
domain specifies its apiType explicitly.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive unit tests for:
- getDefaultApiType(): Tests for single API type detection, multiple
  API types, and fallback behavior
- detectApiTypesFromTemplate(): Tests for all API types and edge cases
- initializeVariables(): Tests verifying lazy evaluation behavior,
  ensuring getDefaultApiType() is only called when needed

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@Mmarzex
Copy link
Contributor

Mmarzex commented Feb 1, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions
Copy link

github-actions bot commented Feb 1, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 1, 2026

📝 Walkthrough

Walkthrough

The ServerlessCustomDomain plugin's API type detection logic was refactored to use lazy evaluation instead of eager computation. The default API type is now computed only when needed and cached for reuse. Comprehensive unit tests were added to verify the behavior.

Changes

Cohort / File(s) Summary
Domain Plugin Refactoring
packages/serverless/lib/plugins/aws/domains/index.js
Simplified getDefaultApiType to return the first detected API type or default to HTTP. Modified initializeVariables to defer defaultApiType computation until required, with caching for subsequent domains.
Comprehensive Unit Tests
packages/serverless/test/unit/lib/plugins/aws/domains/index.test.js
Added full test coverage for getDefaultApiType, detectApiTypesFromTemplate, and initializeVariables, validating lazy evaluation, proper type detection, and correct propagation of explicit vs. default apiType values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • eahefnawy

Poem

🐰 Hops of joy through the code we go,
Lazy loading makes it flow,
No more eager, compute with grace,
Only when needed, the right time, the right place!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: implementing lazy evaluation for handling multiple API types in domain configuration, which is the primary objective of this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@cstea
Copy link
Author

cstea commented Feb 1, 2026

I have read the CLA Document and I hereby sign the CLA

@czubocha
Copy link
Contributor

czubocha commented Mar 2, 2026

Thanks! Closing as this was fixed in v4.32.0

@czubocha czubocha closed this Mar 2, 2026
@github-actions github-actions bot locked and limited conversation to collaborators Mar 2, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.