[core + 2 more] Make LambdaCompute the default compute - #341
Conversation
🦋 Changeset detectedLatest commit: 45c39c5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Simone319
changed the title
f5c7737 to
e9cd30c
Compare
e9cd30c to
01f54c9
Compare
01f54c9 to
6b52df6
Compare
…eway Add a Scope.compute getter resolving in order: explicit block compute (_compute), nearest ancestor scope compute (_scopeCompute), else the owning stack/backend's default compute. Invert resource ownership toward the multi-compute end state: the default LambdaCompute now OWNS the Lambda function + API Gateway. setupBlocksInfra no longer creates them (it keeps the shared role, resource groups, console routes); BlocksStack/BlocksBackend expose handler/gateway/apiUrl as getters delegating to the default compute. The compute is created in create() before the backend import, so a block reading this.compute in its constructor resolves to it. The LambdaCompute value is loaded via a runtime import() to avoid a load-time cycle (LambdaCompute extends ComputeBlock extends Scope). _compute / _scopeCompute are internal (no public option yet). The function + gateway now synth under .../DefaultCompute/..., changing their CloudFormation logical IDs (resource replacement + new API URL on existing stacks). These resources are internal and there is no production usage, so this is a one-time synth change. Implements Multi-Compute A3 (#1019).
There was a problem hiding this comment.
I might need to spend more time with this one to fully grok it. I've left questions for now. Don't block on me there. More importantly, are you coordinating with @osama-rizk on the new defaults option for BlocksBackendProps?
| // In sandbox mode, allow localhost origins so the local dev frontend can | ||
| // reach the deployed Lambda API via CORS. | ||
| const isSandbox = | ||
| this.node.tryGetContext('sandboxMode') === 'true' || this.node.tryGetContext('sandboxMode') === true; | ||
| if (isSandbox) { | ||
| this.fn.addEnvironment('CORS_ALLOWED_ORIGINS', '^https?://(localhost|127\\.0\\.0\\.1)(:\\d+)?$'); | ||
| } |
There was a problem hiding this comment.
This should probably build on defaults mechanism: #302
Please make sure between you and @osama-rizk, depending on which PR merges first, we ultimately avoid sandbox detection in the BB.
| } | ||
|
|
||
| static async create(scope: Construct, id: string, props: BlocksStackProps) { | ||
| static async create(scope: Construct, id: string, props: BlocksStackProps, defaultComputeFactory: DefaultComputeFactory) { |
There was a problem hiding this comment.
If I'm reading this right, this similarly should be part of props.defaults after #302
But, I'm actually slightly lost. This is the BlocksStack.create() method, which appears to be adding a required fourth positional parameter. I would expect that to impact all of the templates and be a breaking change. But, I don't see those changed. Where is my disconnect? 😅
| } | ||
|
|
||
| static async create(scope: Construct, id: string, props: BlocksBackendProps) { | ||
| static async create(scope: Construct, id: string, props: BlocksBackendProps, defaultComputeFactory: DefaultComputeFactory) { |
There was a problem hiding this comment.
I have the same confusion here as I do for BlocksStack.create() with the new required parameter.
2 participants
Problem
With the
Computeabstraction andLambdaComputein place (#329), the framework needs to actually resolve which compute a block runs on and make a stack's default compute own the function + API Gateway — the step that turns the abstraction into the app's real backend.Changes
Scope.computeresolution. Acomputegetter onScoperesolves the compute a block runs on: the nearest_computeassigned on the block or an ancestor scope, else the owning stack/backend's default compute. For any app that never assigns a compute, this always resolves to the default — a no-op refactor.DefaultComputeFactorytype (internal path) and takes a factory as a required argument tocreate(), calling it to build the default without importing a concrete compute class. The umbrella@aws-blocks/blocks— the one package that depends on both core andbb-lambda-compute— suppliesLambdaComputeas that factory through a plainimport, wrappingBlocksStack/BlocksBackend.create()so the customer call site is unchanged. The factory is a positional argument, not a prop, so customers can't set it.LambdaComputenow owns the Lambda function + API Gateway that back a stack'shandler/gateway/apiUrl;setupBlocksInfrano longer creates them, and those stack accessors become getters that delegate to the default compute (typed viaLambdaShapedCompute). The default compute is created increate()before the backend import, so a block readingthis.computein its constructor resolves it.Logical-ID note: because the function + API Gateway now live under the default compute's construct path (
.../DefaultCompute/...), their CloudFormation logical IDs change (a one-time replace of the function + API Gateway, and the API URL changes). These resources are internal, not a customer-facing contract, and there is no production usage yet, so this needs no action beyond redeploy.Alternatives considered
Self-registration via a side-effect import (rejected). An earlier iteration had core keep a module-level factory slot with
registerDefaultComputeFactory(fn), and@aws-blocks/bb-lambda-compute/registerfill it via a side-effectimportthat@aws-blocks/blockspulled in. Rejected in favor of the injectedcreate()argument because:@aws-blocks/blocks→@aws-blocks/bb-lambda-computeis now a normalimportthe module graph, bundler, tree-shaker, andlint:depsall see — not a load-bearing side-effect import that a "remove unused import" cleanup would silently break, surfacing far away increate().create()call, instead of a last-writer-wins singleton shared across every stack in a synth (which the multi-stage/pipeline synths — see the ESM cache-busting tests — are sensitive to).@aws-blocks/corecaller who omits it gets a type error, not a runtime throw at synth.registerClientMiddlewarelooks similar but is mechanically different — an instance method a block calls from its constructor, registering a string into a collector array consumed later by codegen. The computeregisterwas a novel "overwrite a singleton with an executable closure, invoked synchronously" pattern that merely borrowed the name; reusing the name for different semantics is a readability cost, not a consistency win.The tradeoff accepted: the umbrella now wraps
BlocksStack/BlocksBackend.create()(preserving the instance types) instead of re-exporting the classes verbatim. That wrapper must track core'screate()signature by hand, but it's local, boring, and testable — versus an invisible import-order contract.Validation
npm run build:force(full monorepo) green.@aws-blocks/core: 654/654 pass.@aws-blocks/bb-lambda-compute: 11/11 pass (6 CDK-shape + 5Scope.computeresolution).@aws-blocks/blocks: 42/42 pass (conditional-export parity + vendorize-map).npm run lint,npm run lint:deps,npm run check:api,npm run sync-docs:checkall clean.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.