Uh oh!
There was an error while loading. Please reload this page.
docs: note the CORS requirement for CDN-served assets - #352
Conversation
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds an upgrade-guide warning for deployments that serve frontend assets from a separate CDN origin, noting that the ES module move requires CORS headers to allow the site origin.
Changes:
- Adds a new caution/admonition block explaining the ES module + CORS requirement for CDN-served assets
- Clarifies that same-origin (non-CDN) deployments are unaffected
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
xantorres
commented
Aug 4, 2026
Both review points addressed in aab64e0: the requirement is now anchored to its source (the frontend migration PR, linked in place of the versionless phrasing, so a concrete version can be substituted at release time), and origin is defined precisely as scheme, host, and port with an example. |
robinv8
left a comment
There was a problem hiding this comment.
Thanks for documenting this — the CORS requirement for CDN-served ES modules is real and worth calling out.
One concern about placement: docs/getting-started/upgrade.md is the generic upgrade path (Docker pull / binary upgrade / force upgrade). Most readers here are on same-origin deployments and never touch a CDN. A full :::caution block about CDN CORS is deployment/plugin configuration, not an upgrade procedure, so it feels out of place in this guide.
I'd suggest:
- Put the substantive note next to where CDN is already documented — e.g.
docs/getting-started/configfile.md(ui.public_url/ static assets), and/or thecdn-aliyun/cdn-s3plugin READMEs (which already need the concrete CORS rules). - If the upgrade guide should mention it at all, keep it to a short cross-link, e.g. “If you serve static assets from a separate CDN origin, ensure CORS is configured — see Config file / the CDN plugin README.” rather than the full caution block.
Happy to discuss if you had a specific reason to anchor it on the upgrade page (e.g. release-note visibility for operators who only re-read upgrade docs).
xantorres
commented
Aug 5, 2026
@robinv8 Agreed on placement, done in dff3278. The substantive note now lives in the configuration file documentation beside the ui block (as a tip, matching that page's existing convention), and the upgrade guide keeps a single cross-link sentence in the shape you suggested. No strong reason to anchor it on the upgrade page beyond operator visibility, and the cross-link preserves that. |
robinv8
commented
Aug 10, 2026
@xantorres Thanks for the follow-up changes. After re-checking placement, this does not belong in answer-website. CORS is a CDN plugin concern; the long-form note should stay in the plugin READMEs (apache/answer-plugins#326). The Answer release that ships Vite can mention it briefly in release notes. Please close this PR — no need to document this in upgrade/configfile. |
xantorres
commented
Aug 10, 2026
Closing per the placement decision on apache/answer#1567: the CORS note lives in the CDN plugin READMEs (apache/answer-plugins#326), with a short release-notes call-out when the version that ships the migration is cut. Thanks for thinking the placement through to the simpler answer. |
Adds a caution block to the upgrade guide for deployments that serve static assets from a separate CDN origin: with the frontend moving to ES modules in apache/answer#1567, the CDN must send Access-Control-Allow-Origin for the site origin or pages load without JavaScript. Same-origin deployments are unaffected and the note says so explicitly. Requested by review on that PR; wording carries the crossorigin rationale documented there. Companion to apache/answer#1567, best merged with or after it.