Skip to content

Pro 9889 jsx conversion - #565

Open
BoDonkey wants to merge 19 commits into
mainfrom
PRO-9889-jsx-conversion
Open

Pro 9889 jsx conversion#565
BoDonkey wants to merge 19 commits into
mainfrom
PRO-9889-jsx-conversion

Conversation

@BoDonkey

@BoDonkey BoDonkey commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Please indicate which branch this PR should merge into:

Check one

  • main
  • latest
  • stable
  • Check if this PR will be resubmitted against another branch

Summary

Summarize the changes briefly, including which issue/ticket this resolves. If it closes an existing Github issue, include "Closes #[issue number]"
This PR converts a large majority of the templates to JSX. It also introduces a number of fixes. It should not be published until the two starter kit JSX branches land. Closes PRO-9889.

What are the specific steps to test this change?

For example:

  1. Run the website and log in as an admin
  2. Open a piece manager modal and select several pieces
  3. Click the "Archive" button on the top left of the manager and confirm that it should proceed
  4. Check that all pieces have been archived properly

What kind of change does this PR introduce?

(Check at least one)

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Build-related changes
  • Other

Make sure the PR fulfills these requirements:

  • It includes a) the existing issue ID being resolved, b) a convincing reason for adding this feature, or c) a clear description of the bug it resolves
  • The changelog is updated
  • Related documentation has been updated
  • Related tests have been updated

If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

@BoDonkey
BoDonkey requested review from boutell and a lite review from Copilot September 2, 2026 14:28
@linear

linear Bot commented Sep 2, 2026

Copy link
Copy Markdown

PRO-9889

@boutell boutell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial feedback. Recommend you back out the unplanned moves of files (which are showing up as deletions and creations) and address the points around needing to show how access to global and home is actually obtained, then re-request review

@@ -1,336 +0,0 @@
---

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this die?

@BoDonkey BoDonkey Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not linked from anywhere, but will be cleaned up in another PR. Reverted.

| `APOS_BASE_URL` | Astro staging frontend URL (set after Part 2) | `https://your-project.vercel.app` |
| `NODE_ENV` | Sets production mode | `production` |
| `APOS_RELEASE_ID` | Unique ID per deploy for cache busting | `${{RAILWAY_GIT_COMMIT_SHA}}` |
| `APOS_RELEASE_ID` | Unique ID per deploy for cache busting | <span v-pre>`${{RAILWAY_GIT_COMMIT_SHA}}`</span> |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the funny span with v-pre thing for this and not other columns?

@BoDonkey BoDonkey Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vitepress tries to interpret double curly braces through mustache interpolation. The wrapping prevents that. This sweep of the docs picked up a few that missed being escaped.

> **`APOS_EXTERNAL_FRONT_KEY`:** Generate a long random string for this value. It authenticates the Astro frontend with ApostropheCMS. Both Railway and Vercel must use the exact same value or requests will be rejected. Treat it like a password.

> **`APOS_RELEASE_ID`:** Setting this to `${{RAILWAY_GIT_COMMIT_SHA}}` uses Railway's built-in reference variable to automatically generate a unique value from the git commit hash on every deploy. This is preferable to a static random string, which would remain the same across redeployments.
> **`APOS_RELEASE_ID`:** Setting this to <span v-pre>`${{RAILWAY_GIT_COMMIT_SHA}}`</span> uses Railway's built-in reference variable to automatically generate a unique value from the git commit hash on every deploy. This is preferable to a static random string, which would remain the same across redeployments.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question each time this appears

For filter links, prefer `aposData.filters` (derived from `req.data.filters`) over the legacy `aposData.piecesFilters` pattern where possible. This provides filter metadata and pre-built URLs that are consistent with how static paths are generated.

Note that static mode only supports a single active filter at a time. Combined filters will not have pre-rendered paths and will return 404s. See the note in [Backend Configuration](#3-configure-piece-filters-for-static-generation) above.
Note that static mode only supports a single active filter at a time. Combined filters will not have pre-rendered paths and will return 404s. See the note in [Backend Configuration](#_3-configure-piece-filters-for-static-generation) above.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sanity checking that _3 is correct? I mean maybe

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup - the anchor link is numbered and #3-XXX is invalid in, get this, CSS. So, VitePress slugifies during anchor link creation to make them valid.

Comment thread docs/tutorials/building-navigation.md Outdated
{# 👇 Referencing `data.home._children` and looping over them. #}
{% for page in data.home._children %}
{/* 👇 Referencing `home._children` and looping over them. */}
{home._children.map((child) => (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not show how we got hold of home and global in the first place. A more complete example that does that is called for

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed here and in one other instance on guide/pages.md.

@boutell

boutell commented Sep 2, 2026

Copy link
Copy Markdown
Member

(global and home are available the same way page is available)

As a matter of fact they are properties of the data prop given to the page template, just like in Nunjucks, so it's extra strange the way claude wrote it here without capturing that to a variable first

@BoDonkey
BoDonkey requested a review from boutell September 3, 2026 10:18

@boutell boutell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 😄 I did a bunch of spot checking; it's rather epic, of course, but feels good to me.

@boutell

boutell commented Sep 4, 2026

Copy link
Copy Markdown
Member

Let me know when this lands, I want to bring in WYSIWYG Fields documentation after that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants