docs(self-hosting): restore preview banner - #8628
Conversation
Put back the preview callout and the '(Preview)' section title that were removed during review of #8605. ampx deploy still ships as a preview release, so the docs should say so. Also adds the banner to the secrets and environment variables page, which was added after the banner was removed.
bobbor
left a comment
bobbor
left a comment
There was a problem hiding this comment.
The banner text itself is fine. I'm requesting changes because the PR flips a deliberate decision without addressing the evidence that drove it, and leaves the one place preview status is operationally load-bearing contradicting the restored banner.
Which signal is authoritative?
Two things argue that this shipped rather than stayed in preview:
npm view @aws-amplify/hostinggiveslatest: 1.0.1— a stable semver on the default tag, with nopreview/nextdist-tag. Sonpm add @aws-amplify/hosting(getting-started/index.mdx:59) resolves a reader straight to 1.0.1, which is hard to square with "APIs may change before general availability". A 1.0.x onlatestis itself an API-stability promise.- RFC aws-amplify/amplify-backend#3211 is closed as COMPLETED (2026-09-11, ~1h after #8605 merged) — which is why the second commit here had to repoint the link.
The evidence in your description (the PREVIEW release — not intended for production use prompt and the install notice) is real and user-facing, so I'm not arguing the banner is wrong — I'm arguing the contradiction lives upstream, and the docs shouldn't be where we resolve it by picking a side. If the feature is genuinely preview, the fix is upstream: a preview dist-tag and a 0.x version. If it's GA, the CLI prompt and install notice are stale leftovers to remove. Could you confirm with the hosting team which it is and link that here? Whichever way it lands, the docs should then match all three surfaces, not two of three.
Two claims in the description don't hold
"Both were removed during review of #8605." No reviewer asked for this. I read all 20 inline comments on #8605 — none mentions preview, GA, or production framing. The removal was your own commit b0a72bdf, under a "Release framing" heading with an explicit rationale: "remove preview banners and 'not for production' language; drop stale 'install the preview via RFC' instruction (ships as 1.0.0 on npm)". That rationale is still true (it's 1.0.1 now), and this PR reverses it without mentioning it. Please state what new information changed the conclusion — that's the substance of the review.
"Banner restored verbatim." It isn't. The original read "This feature is in preview and part of our RFC. ... See the RFC for instructions on installing the preview release." The new text drops the RFC reference and the install pointer, and ends with "Share feedback or report issues on GitHub". That's a reasonable edit given the RFC closed, but "verbatim" invites reviewers to skip reading it.
| {/* PREVIEW-BANNER-START */} | ||
| <Callout warning> | ||
|
|
||
| **This feature is in preview.** It is not recommended for production workloads yet, and APIs may change before general availability. Share feedback or report issues on [GitHub](https://github.com/aws-amplify/amplify-backend/issues). |
There was a problem hiding this comment.
This PR restores "not recommended for production workloads" but leaves the one page where that status is operationally load-bearing untouched.
Line 53 of this file tells every reader to Pass --yes in every CI job because "ampx deploy prompts for confirmation before running" — and per your description that prompt is specifically the "PREVIEW release — not intended for production use" acknowledgment. So the page now teaches users to auto-accept a not-for-production warning, in jobs whose every example is --identifier production (lines 81, 84, 109-110, 125-126, 158, 174, 183; also getting-started/index.mdx:68, 80).
If the banner goes back, these need reconciling in the same PR: describe at line 50/53 what the prompt actually says, and either rename the example identifiers away from production or add a line acknowledging the tension. Otherwise the banner reads as decoration that the rest of the page ignores.
There was a problem hiding this comment.
Agreed, fixed in d7c493c. The flags section now quotes the prompt (ampx deploy is a PREVIEW release — not intended for production use…) and says that --yes accepts it without changing the preview status, so deployments should stay non-production until GA. I also renamed the example identifiers from production to my-app here and in Getting started.
| }; | ||
| } | ||
|
|
||
| {/* PREVIEW-BANNER-START */} |
There was a problem hiding this comment.
Seven byte-identical copies with no single source. The PREVIEW-BANNER-START/END markers are a good instinct, but nothing in the repo consumes them (this PR introduces the only instances — there's no precedent for that marker style in src/pages), so they're a convention on trust. This PR is already the second drift event: the banner was dropped in b0a72bdf and the secrets page was then added without it, which is why you're adding a 7th copy now.
Given the whole point is that this text gets deleted in one shot at GA, could this be a single shared component (e.g. <PreviewBanner /> in src/components, which needs no MDX import here the way Callout doesn't)? Then GA is one deletion, not seven, and a new page can't silently miss it.
There was a problem hiding this comment.
Good call, done in d7c493c. There's now a shared <PreviewBanner /> in src/components/PreviewBanner, registered in mdx-components.tsx next to Callout (no import needed), with a small unit test. All 7 pages use it, and the unused marker comments are gone. GA removal is now one change.
|
|
||
| export const meta = { | ||
| title: 'Self-managed hosting', | ||
| title: 'Self-managed hosting (Preview)', |
There was a problem hiding this comment.
(Preview) in meta.title has no precedent in this repo — it's the only such title in src/pages, and the one other in-preview feature (build-a-backend/aws-blocks/index.mdx) marks preview status in prose and a callout only, leaving its title clean. meta.title feeds the <title>, the sidebar label, and the breadcrumb, so this is the one change here with an SEO/URL-adjacent footprint and the one hardest to unwind cleanly at GA.
Also, only the parent page gets the suffix — the six child pages keep clean titles, so a reader landing on getting-started from search sees no preview marker in the tab title. Either both or neither; I'd suggest neither, and let the banner carry it.
There was a problem hiding this comment.
Agreed, I removed (Preview) from meta.title in d7c493c, so no page title carries the suffix and the banner states the status on every page.
| } | ||
|
|
||
| {/* PREVIEW-BANNER-START */} | ||
| <Callout warning> |
There was a problem hiding this comment.
Minor: <Callout warning> here puts two warning-styled callouts in the first screenful — this one and the actionable CDK-bootstrap prerequisite at line 43. The bootstrap one is a thing the reader must do; the preview notice is context. The AWS Blocks page uses <Callout info> for exactly this kind of preview notice. Suggest info so the warning styling still means "act on this".
There was a problem hiding this comment.
Agreed, the banner component uses <Callout info> (matching the AWS Blocks notice), so the CDK bootstrap prerequisite is the only warning callout on the page. Done in d7c493c.
…, reconcile --yes - Replace the seven inline banner copies with a shared <PreviewBanner /> component registered in mdx-components (like Callout), so it can't drift and removal at GA is a single change. Drops the unused PREVIEW-BANNER marker comments. - Use an info callout (matching the AWS Blocks preview notice) so warning styling stays reserved for actionable notes like the CDK bootstrap prerequisite. - Drop '(Preview)' from the section title; the banner carries the status. - External pipelines: describe what the ampx deploy prompt actually says, note that --yes accepts it without changing preview status, and rename example identifiers from 'production' to 'my-app' (also in getting started).
bobbor
left a comment
bobbor
left a comment
There was a problem hiding this comment.
All four inline findings are fixed at d7c493c6, and I verified them against the head blobs rather than the commit message: the shared <PreviewBanner /> replaces all seven copies and follows the existing PinpointEOLBanner/LexV1EOLBanner pattern (Callout + next/link + __tests__, registered in mdx-components.tsx), the markers are gone, meta.title no longer carries the suffix, the banner is an info callout so the CDK bootstrap prerequisite is the only warning left, and no --identifier production remains anywhere in the section.
I also withdraw my "which signal is authoritative" objection, because I checked the published artifacts and your premise is stronger than the description claims:
@aws-amplify/backend-cli@1.10.0(thelatesttag) carries the prompt verbatim atlib/commands/deploy/deploy_command.js:129, guarded byif (!args.yes).@aws-amplify/hosting@1.0.1declares"postinstall": "node -e \"console.log('...PREVIEW release — not for production use')\""in its own publishedpackage.json, and its README mentions preview nowhere.
So two stable-versioned packages on latest each self-declare PREVIEW at runtime; the preview signals are not confined to the test/iac-hosting prerelease tags. There is no side for the docs to pick — describing what a user actually encounters is right, and the --yes prose is the honest way to handle the one path where the warning is skipped.
One thing left for a follow-up rather than this PR: nothing points at where the contradiction gets reconciled upstream (either a preview dist-tag plus a 0.x, or removing the prompt and the postinstall hook). Worth an issue on amplify-backend so the banner has a defined removal trigger.
Non-blocking nit, take it or leave it: external-pipelines/index.mdx:187 now maps main to my-app, so the environment-specific example names an app rather than an environment; my-app-prod/my-app-staging would keep its point.
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
3 participants
Marks the self-managed hosting section as preview again, using a single shared banner.
The preview notice was removed in my own commit
b0a72bdon #8605, not because a reviewer asked. My reason then was that the package ships on npm (@aws-amplify/hosting@1.0.xonlatest). What that missed is that the product still declares itself preview in two places users see:ampx deployprompts "ampx deploy is a PREVIEW release — not intended for production use. Do you want to continue?" on every run.@aws-amplify/hostingprints a preview notice on install.Every self-hosting path goes through
ampx deploy(local, external CI, anddefinePipeline()viaampx deploy --pipeline), so the docs should match those. The managed Amplify Hosting section is GA and unchanged.Changes
<PreviewBanner />component (src/components/PreviewBanner), registered inmdx-components.tsxlikeCallout, used on all 7 self-hosting pages. GA removal is one change, and a new page can't silently miss it.infocallout style, matching the AWS Blocks preview notice, sowarningstays reserved for actionable notes.ampx deployprompt says and that--yesaccepts it without changing preview status. Example identifiers renamed fromproductiontomy-app(also in Getting started).