Skip to content

Commit a695afb

Browse files
committed
test(core): address review feedback on #173 fix
- Add a static-only (no `api`) test asserting the placeholder is registered as a no-cache path — guards against a regression that makes the registration depend on `props.api`. - Rewrite the invalidation-path test to assert via `Template.hasResourceProperties` + `Match` so a future CDK property rename fails loudly instead of silently skipping the assertion. - Trim the step-5a comment to the non-obvious invariant (config.json is mutable and must not inherit the content-hashed-asset s-maxage); the bug forensics already live in the fix commit body. - Drop a redundant section-divider comment inside the test file.
1 parent c152936 commit a695afb

2 files changed

Lines changed: 36 additions & 33 deletions

File tree

packages/core/src/hosting.test.ts

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,7 +1508,6 @@ describe('Hosting', () => {
15081508
});
15091509
});
15101510

1511-
// ── #173: stale placeholder config.json never served long-term ──
15121511
describe('config.json stale-placeholder guard (#173)', () => {
15131512
// Helper: pull every BucketDeployment custom resource's Properties.
15141513
const bucketDeployments = (stack: Stack) => {
@@ -1571,6 +1570,28 @@ describe('Hosting', () => {
15711570
);
15721571
});
15731572

1573+
it('registers the placeholder as a no-cache path even for a static-only site (no api)', () => {
1574+
// The placeholder is always written (step 5), so its no-cache
1575+
// registration must not depend on `props.api`. This guards against a
1576+
// regression that moves the registration inside an `if (props.api)`
1577+
// block, which would reopen the stale-placeholder window for
1578+
// static-only sites.
1579+
createSpaBuildOutput(tmpDir);
1580+
const app = new App();
1581+
const stack = new Stack(app, 'StaticOnlyPlaceholderStack');
1582+
new Hosting(stack, 'Hosting', { root: tmpDir });
1583+
1584+
Template.fromStack(stack).hasResourceProperties(
1585+
'Custom::CDKBucketDeployment',
1586+
Match.objectLike({
1587+
Include: Match.arrayWith(['.blocks-sandbox/config.json']),
1588+
SystemMetadata: Match.objectLike({
1589+
'cache-control': 'no-cache, no-store, must-revalidate',
1590+
}),
1591+
}),
1592+
);
1593+
});
1594+
15741595
it('invalidates the post-rewrite cache key (/builds/<id>/.blocks-sandbox/*)', () => {
15751596
// The viewer-request skew-protection function rewrites the URI to
15761597
// `/builds/<buildId>/.blocks-sandbox/config.json` BEFORE the cache
@@ -1581,28 +1602,15 @@ describe('Hosting', () => {
15811602
const stack = new Stack(app, 'InvalidationPathStack');
15821603
new Hosting(stack, 'Hosting', { root: tmpDir, api: MOCK_API });
15831604

1584-
const deployments = bucketDeployments(stack);
1585-
// The BlocksConfigDeployment is the one targeting the .blocks-sandbox
1586-
// key prefix with an invalidation configured.
1587-
const configDeploy = deployments.find(
1588-
(p) =>
1589-
typeof p.DestinationBucketKeyPrefix === 'string' &&
1590-
(p.DestinationBucketKeyPrefix as string).endsWith('.blocks-sandbox') &&
1591-
Array.isArray(p.DistributionPaths),
1592-
);
1593-
assert.ok(
1594-
configDeploy,
1595-
'expected a BlocksConfigDeployment with distributionPaths',
1596-
);
1597-
1598-
const paths = configDeploy.DistributionPaths as string[];
1599-
const matchesRewrittenKey = paths.some((p) =>
1600-
/^\/builds\/.+\/\.blocks-sandbox\/\*$/.test(p),
1601-
);
1602-
assert.ok(
1603-
matchesRewrittenKey,
1604-
`distributionPaths must invalidate the post-rewrite cache key ` +
1605-
`(/builds/<id>/.blocks-sandbox/*); found ${JSON.stringify(paths)}`,
1605+
// Assert via Template + Match so a CDK property rename fails loudly here
1606+
// rather than silently skipping a structural-heuristic lookup.
1607+
Template.fromStack(stack).hasResourceProperties(
1608+
'Custom::CDKBucketDeployment',
1609+
Match.objectLike({
1610+
DistributionPaths: Match.arrayWith([
1611+
Match.stringLikeRegexp('^/builds/.+/\\.blocks-sandbox/\\*$'),
1612+
]),
1613+
}),
16061614
);
16071615
});
16081616
});

packages/core/src/hosting.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -501,17 +501,12 @@ export class Hosting extends Construct {
501501
);
502502

503503
// ── 5a. Mark config.json as a no-cache path ─────────────────────
504-
// config.json is a fixed-name, mutable runtime-config file it must
504+
// config.json is a fixed-name, mutable runtime-config file: it must
505505
// NOT inherit the content-hashed mutable-asset cache-control
506-
// (`s-maxage=31536000`) the static-asset deployment applies to
507-
// `/assets/<hash>.js` and friends. If it did, an edge that cached the
508-
// build-time *placeholder* (`{_placeholder:true}`) during the deploy
509-
// window would keep serving it for up to a year — `getApiUrl()` then
510-
// throws and every client API call fails. Registering it as a
511-
// no-cache path routes the placeholder into the `no-cache, no-store,
512-
// must-revalidate` deployment tier so the edge never caches it
513-
// long-term; the real config is deployed in step 8 with its own
514-
// short `max-age=60`.
506+
// (`s-maxage=31536000`) applied to `/assets/<hash>.js`. Registering it
507+
// as a no-cache path uploads the build-time placeholder with
508+
// `no-cache, no-store, must-revalidate` so an edge never caches it
509+
// long-term; the real config is deployed in step 8 with `max-age=60`.
515510
const configNoCachePath = `${BLOCKS_SANDBOX_DIR}/config.json`;
516511
const existingNoCachePaths = manifest.staticAssets.noCachePaths ?? [];
517512
manifest.staticAssets.noCachePaths = existingNoCachePaths.includes(configNoCachePath)

0 commit comments

Comments
 (0)