yoastseo package: define serializable PaperDTO schema and conversion function#23336
Conversation
Coverage Report for CI Build 1Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.2%) to 57.64%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions73 previously-covered lines in 9 files lost coverage.
Coverage Stats💛 - Coveralls |
Adds a root `contract/` redirect mirroring `yoastseo/researcher` so consumers import `yoastseo/contract` instead of deep-requiring `build/`. Keeps zod off the package-root bundle (only pulled when the contract is imported) and avoids an `exports` map, which would break existing deep importers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
…d and remove siteUrl/domain placeholders
…r non-WordPress consumers
…ress-transitional fields for analysis parity
…igration-define-a-serializable-paperkeyphrase-input-contract-for-non-wordpress-consumers
…d improve documentation
…ict extension Addresses PR review: replace lodash.isUndefined with nullish coalescing for the keyphrase/keyword alias (no extra dependency, avoids the no-undefined rule), and note in createToPaper's JSDoc that .extend() preserves .strict() so open-ended extra keys need .passthrough(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ion for consumer-defined assessments
|
/build-zip |
|
📦 Plugin zip built successfully! Download it from the workflow run. |
There was a problem hiding this comment.
- When building I get changes in the yarn.lock files, should be added to this PR?
- Should the change log entry include the version increase? like [yoastseo 0.1.0]?
- Lint warnings were reduced to 18, lets update the package.json (now its 19)
- when doing
yarn buildinapp/content-analysis-webworkerI get:
warning package.json: No license field
$ webpack
/bin/sh: webpack: command not found
error Command failed with exit code 127.
Some feedback from Claude that seems valid:
- createToPaper is promised but not delivered
The PR description calls out extensibility as a key feature:
▎ "Extensible by consumers via createToPaper(paperDtoSchema.extend({…}))"
But createToPaper is not exported anywhere in the diff — only toPaper and paperDtoSchema. A consumer who wants to extend must know the internal mapping (keyphrase → keyword, etc.) to rebuild the attributes object themselves, which defeats the stated goal. Either implementit or drop the claim from the description.
- Routes read request.body.locale directly after the contract validated it
In every route handler, after paperFromRequest validates the body through Zod, the code still reads the rawrequest body:
const paper = paperFromRequest( request, response ); // Zod validates here
if ( ! paper ) { return; }
const language = request.body.locale || "en"; // raw body again — bypasses the contractpaper.getLocale() returns the validated locale (e.g. "en_US"). The language prefix could be extracted from that. As-is, the contract validates the locale field but the routes ignore the validated value and reach back into the raw body, which is inconsistent.
- meta-description and seo-title routes check request.body before the contract
// analyze.js — /analyze/meta-description
if ( ! request.body.description ) {
return response.status( 400 ).json( { error: "Description is required" } );
}
const paper = paperFromRequest( request, response ); // Zod runs afterBut /analyze/keyphrase does it the right way — contract first, then paper.hasKeyword(). The meta-description and seo-title routes do a falsy check on the raw body before Zod has had a chance to type-check. The pattern should be consistent: validate through the contract first, then inspect the resulting Paper via its accessors.
- Test accesses private _attributes directly
// paperDtoSpec.js
expect( paper._attributes.wpBlocks ).toEqual( wpBlocks );_attributes is internal state. If Paper has no public getter for wpBlocks, the test should assert behaviour(e.g. that the resulting analysis score changes) rather
than poking at private state. This will break silently if Paper's internals are ever restructured.
- undefined values passed explicitly to Paper
const attributes = {
keyword: keyphrase, // undefined when neither keyphrase nor keyword supplied
synonyms: data.synonyms, // undefined
locale: data.locale, // undefined
...
};
return new Paper( data.text, attributes );The attributes object is built with explicit undefined values for absent optional fields. This is different from omitting the keys entirely. Paper's constructor
likely handles undefined gracefully, but it's fragile — if it ever iterates Object.keys(attributes), it would see all keys, not just the ones with values. A defensive approach:
Object.fromEntries( Object.entries( attributes ).filter( ( [ , v ] ) => v !== undefined ) )- GET endpoints with request bodies (pre-existing, but worsened)
All routes use app.get(...) but read request.body. GET bodies are not part of the HTTP spec and are stripped by many proxies and CDNs. This is pre-existing, but this PR cements the pattern by wrapping it in paperFromRequest. Worth tracking — the right fix would be to switch these to POST, which is a separate task.
…n for consistency
|
Thanks for the review! Addressed in the latest push. Build/process questions
Code findings
|
Context
The API-driven content-analysis migration needs a documented, serializable input that the analysis engine can validate, so non-WordPress consumers (a hosted web API, the Shopify app, the Google Docs extension) stop hand-rolling
Paperinput and each shipping their own variant of input bugs (e.g. the ShopifysiteUrl/domainconfusion in Yoast/lingo-other-tasks#97).This PR adds that input contract — a serializable
PaperDTOplus atoPaperboundary that validates it and constructs the engine's internalPaper— exposed as an opt-inyoastseo/contractentry, and converts the in-repocontent-analysis-apireference app to use it as the first consumer. The WordPress plugin is intentionally untouched: it keeps constructingPaperdirectly.Summary
This PR can be summarized in the following changelog entry:
PaperDTOinput contract andtoPaperboundary, exposed via theyoastseo/contractentry point, for validating and mapping analysis input.Relevant technical choices:
yoastseo/contractentry (mirroringyoastseo/researcher) rather than from the package root. The plugin loadsyoastseoas a webpack external/global, so keeping the contract off the root keeps zod out of the WordPress bundle — only consumers that import the contract pay for it. Noexportsmap was added (it would break existing deep importers of the openbuild/tree); the entry is a root redirect dir listed infiles.keyphraseis canonical;keywordis accepted as a deprecated alias. Every current consumer speaks the engine'skeyword, so the alias lets them adopt without renaming;keyphraseis the name we steer toward.wpBlocks/shortcodes/isFrontPage) are included as optional + deprecated, despite the goal of a neutral contract. They are real analysis inputs that change WordPress scores (shortcodes are stripped before counting/matching; blocks drive tree construction), so a remote/API analysis can only reproduce in-browser scores if it can send them. They are marked deprecated pending the neutral structured-content work (#264).siteUrl/domaindeferred. No consumer feeds them throughPapertoday and no assessment reads them (competing-links gets the site URL from context); they’ll be added with that assessment’s refactor so the semantics are shaped against a real reader.customDatakept open (z.record, shape-checked but contents unvalidated) to avoid coupling the contract to platform/product (Shopify/WooCommerce) shapes.Test instructions
Test instructions for the acceptance test before the PR gets merged
This is a developer-facing change to the
yoastseopackage (no UI). It can be verified as follows:packages/yoastseo, run the contract unit tests usingyarn test [full path to paperDtoSpec.js].yarn build) and confirm the public entry resolves:node -e 'const { toPaper } = require("yoastseo/contract"); console.log(toPaper({ text: "A cat post", keyphrase: "cat food" }).getKeyword());'printscat food.toPaper({ text: 123 })andtoPaper({ text: "x", keyphrse: "typo" })both throw.apps/content-analysis-webworker, runyarn install && yarn build(the app is not part of the root Yarn workspace, so its webpack devDependency must be installed first) — it compiles with no errors, exercisingimport { toPaper } from "yoastseo/contract"through the bundler (thecontent-analysis-apiconsumer only exercises the Noderequirepath).packages/jschanges in this PR).Relevant test scenarios
Test instructions for QA when the code is in the RC
Not applicable — this is a non-user-facing package change with no RC-visible behaviour.
Impact check
This PR affects the following parts, which may require extra testing:
packages/yoastseo: a new opt-inyoastseo/contractentry andzodadded as a dependency (install-size only; the contract is off the package root, so the WordPress bundle does not gain zod). The bundle impact for consumers that do import the contract has not been measured with the analyzer — flagged for follow-up.apps/content-analysis-api: routes refactored to buildPapervia the contract (now returns a 400 on structurally invalid input instead of silently analysing it). In-repo reference app; no plugin impact.apps/content-analysis-webworker: the browser/webpack demo now builds itsPapervia the contract too — both confirms theyoastseo/contractentry resolves and bundles client-side, and serves as the second reference consumer. In-repo demo; no plugin impact.packages/js): no changes — analysis input is built exactly as before.Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.[yoast-doc-extension], added test instructions for Yoast SEO for Google Docs and attached theGoogle Docs Add-onlabel to this PR.Documentation
PaperDTOentry, and JSDoc on the schema/mapper.)Quality assurance
grunt build:imagesand committed the results, if my PR introduces or edits images or SVGs. (No images.)Innovation
innovationlabel.Fixes Yoast/lingo-other-tasks#634