[pull] main from TryGhost:main - #1462
Merged
Merged
Conversation
no ref - add Node 24 to test leg now that sqlite3 is removed from the test matrix
…#30435) no ref pnpm installs the Node pinned by `devEngines.runtime` and links its bin ahead of the one setup-node installs, so both legs of the unit, legacy and acceptance matrices ran 22.23.1 and the Node 24 legs proved nothing. Passing `--no-runtime` to those installs skips the runtime dependency, so each leg runs the version its matrix entry asked for. That makes two genuine Node 24 failures visible, fixed here so the matrix goes green on both legs: - `res._headers` was removed in Node 24, so the mock-express helper handed every assertion an undefined headers object. - Node 24 adds a literal `module.exports` key to a CJS module's namespace. The external-globals plugin re-exports every key, emitting `export const module.exports = React.module.exports;`, which rolldown rejects and which failed every admin build.
no ref The React editor will need the snippet save/insert feature that Ember Data currently serves in the Ember editor. This adds a snippets API module to `apps/admin-x-framework` with `useBrowseSnippets`, `useAddSnippet`, `useEditSnippet`, and `useDeleteSnippet`.
no ref The acceptance lanes (test:ci:e2e, test:ci:integration) wrapped vitest in c8, which reports coverage V8 can describe but c8 cannot interpret. Two measured problems, both on the real e2e suite: - Branch and function counts are inflated. V8 emits block ranges; without an AST c8 cannot see an `if` with no `else`, a ternary, or a `||`/`??` short-circuit as a branch point at all, so it scores them covered. e2e branches read 80.48% under c8 against 58.00% under the v8 provider, on near-identical denominators (20811 vs 21926) — the gap is entirely in the numerator. - Denominators collapse for files a run never loads. c8 credits each such file exactly 1 branch and 1 function regardless of content. The integration lane leaves most of core/ unloaded, so its branch denominator falls to 12532 against the v8 provider's 21926, and an untested file added today barely moves the gate. Vitest 4's v8 provider does AST-aware remapping, so both go away. Coverage is now configured in vitest.config.db.ts and the lanes just pass --coverage; cobertura still lands at coverage-e2e/ and coverage-integration/, so ci.yml and the nx outputs are unchanged. Thresholds are re-baselined from measured runs against MySQL 8.0 with Redis and MinIO up, minus roughly 2pt of headroom. The branch gates drop hardest (e2e 75 to 56, integration 73 to 32). No test changed and none started failing — the old numbers were simply not measuring what they claimed. Both lanes' gates and report directories live in one COVERAGE_LANES table in the config, selected by the COVERAGE_LANE env var the test:ci:* scripts set, rather than split between the config and a long CLI override. The lookup uses Object.hasOwn, so 'constructor' and the rest of Object.prototype are rejected instead of resolving to inherited members and silently skipping the gate, and an empty string is a bad value rather than "unset". An unset lane reports without gating, so ad-hoc local --coverage runs no longer inherit e2e's thresholds. Vitest 4 has no coverage.all — `include` is what reports never-loaded files. Verified: identical counts with and without it, so it was dead config implying a behaviour `include` already provided. Two entries from .c8rc.e2e.json did not carry over: the `!core/server/data/migrations/utils.js` negation, which matched nothing (utils is a directory), and .c8rc.json, which nothing referenced.
no ref The React editor work needs create/edit mutations for posts and pages whose requests match what the Ember editor sends today, so the API behaves identically for both clients.
…aceholder (#30427) no ref Adds the cutover seam for serving the editor from the React admin with a placeholder and labs flag for future work. This is deliberately 'empty'/non-functional.
…load The definitions hook suppressed the error toast but still let react-query retry the failed request whenever a new consumer mounted. While a retry is in flight the hook reports neither data nor an error, so every consumer saw the definitions disappear and reappear on each mount. On the members page that swing rebuilt the field catalog, rewrote the URL, remounted the filter bar, and kicked off another retry, so a single failed fetch turned into an endless loop. Holding the failure for the life of the cache entry keeps the page in its degraded-but-stable state with one attempt per visit.
… Core An Admin deployed ahead of a Core without the custom field definitions endpoint has to keep filtering intact. Adding a filter swaps the filter bar between its two placements, and each placement asks for the definitions; if the failed answer does not hold across that swap, every swap asks again, the answer flips the page's field catalog, and the URL is rewritten in a loop that never settles. The previous commit fixed that loop but nothing pinned it, so this test drives the filter UI against a 404 definitions endpoint and asserts no further definitions requests are made and the route stays put.
#30440) @acburdine @claude Changed the Ghost-CLI job to run each scenario on its own Node version cbfd782 no ref The job runs two scenarios: installing this build's tarball, and upgrading to it from the newest Ghost on npm. They don't need the same Node version, and once NODE_VERSION moves ahead of what the published release supports they can't share one — Ghost-CLI reads that release's `engines` and refuses, which is correct behaviour and not something the branch can fix. A matrix keeps the job id, so the required-checks aggregator needs no change. The clean-install leg follows NODE_VERSION, since it is what proves this build installs and boots there. The latest-release leg stays on the Node version the published release declares, and moves up once a release ships supporting a newer line. The debug-log artifact is named per scenario, since both legs upload on failure and upload-artifact rejects a duplicate name within a run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )