Fix/domains multiple api types lazy evaluation#13310
Fix/domains multiple api types lazy evaluation#13310cstea wants to merge 2 commits intoserverless:mainfrom
Conversation
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 <[email protected]>
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 <[email protected]>
✅ 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. |
|
All contributors have signed the CLA ✍️ ✅ |
📝 WalkthroughWalkthroughThe 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
I have read the CLA Document and I hereby sign the CLA |
Summary
getDefaultApiType()so it's only called when a domain actually needs a default API typeapiType, no API type detection is performed, avoiding the erroneous "Multiple API typesdetected" error
Problem
Previously,
getDefaultApiType()was called unconditionally duringinitializeVariables(). 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
apiTypespecified, making it impossible to usemulti-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
apiTypeis not explicitly specified). The result is cached to avoid redundant computations.Test plan
getDefaultApiType(),detectApiTypesFromTemplate(), andinitializeVariables()npm run lintpassesnpm run prettierpasses🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes & Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.