Refresh Project#527
Open
kadamwhite wants to merge 30 commits into
Open
Conversation
Companion to modernization-plan.md (the vision): a concrete, sequenced execution plan broken into independently-mergeable phases, each with its own regression net. Records the decisions made during planning. Locked decisions: - Bundler: tsdown (Rolldown), replacing webpack + Grunt + Babel - Native fetch as the default transport; Node 18+; drop IE11 - Remove superagent entirely (no legacy adapter) - Batteries-included main export (restores v1.x ergonomics) - Build-time pre-parsed route tree to cut startup parsing Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The integration suite asserts against the classic "Theme Unit Test" dataset, previously supplied by a separate Chassis VM (kadamwhite/wpapi.local). Replace that with a reproducible wp-env seed: - Commit the WXR as a fixture (tests/fixtures/theme-unit-test-data.xml). - Add `npm run env:seed`: empties the site (drops the default "Hello world!" post and "Sample Page" for determinism), then imports the WXR. - Install the WP-API/Basic-Auth plugin via .wp-env.json so authenticated tests work -- modern WP core rejects plain username/password over REST. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two coupled changes across the integration suite: - Endpoint is no longer hardcoded to http://wpapi.local/wp-json. It comes from tests/helpers/constants.js, which reads WPAPI_HOST (default the wp-env address http://localhost:2747) and derives the /wp-json endpoint. Pagination and upload-URL assertions build off the same constants. - Expected data values updated to match modern WordPress. The WXR "Scheduled" post was future-dated when these tests were written; today it is published, which grows the post set 38 -> 39 and shifts pagination and date-filter results. Modern WP also exposes more settings keys and defaults to an empty tagline. Only expected values changed; every assertion still verifies the same behavior (pagination, filtering, ordering, CRUD round-trips). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WPAPI.discover() read the API root's _links.self as a bare string, but current
WordPress returns an array of link objects ([ { href } ]). Accept either shape.
discover() still cannot bootstrap fully against modern WP: it builds a route
tree from the live route list, which now includes routes whose named groups
contain nested patterns (e.g. wp/v2/templates/(?P<id>...)), and the route-tree
regex parser throws "Unterminated group". That parser work is scoped to phase 5,
so the discover suite is skipped with a TODO. Its beforeAll .catch()es the
rejection so the skipped-but-still-executed hook can't leak into other suites.
Default-mode instances are unaffected -- they use the bundled default-routes.json.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- handoff.md: living status doc; records the Phase 0 baseline, the deferred discover/route-parser gap, and the dev workflow. - modernization-plan.md: backlog note to add a secondary block/FSE-oriented seed in phase 5 (the classic WXR predates the block editor), kept separate from the classic dataset that guards the wp/v2 regression net. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Establishes a reproducible local integration environment (wp-env + seeded Theme Unit Test data + Basic-Auth) and gets the full suite green on modern WordPress: 805 passed, 16 skipped (discover, deferred to phase 5), 0 failed. No request/library logic changed except an isolated discover() self-link compatibility fix.
Call out moving all automation to GitHub Actions as a first-class concern: ci.yml (phase 1), docs.yml deploy (phase 6), and release.yml publish. Replaces Travis and the hand-run docs/release scripts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Swaps the browser/Node build entirely for tsdown (Rolldown-based): dual
ESM+CJS for the `index`/`fetch`/`superagent` entries plus a browser UMD
bundle per transport, with generated .d.ts and package.json `exports` map.
Each Node entry gets its own independent tsdown config rather than one
shared multi-entry config, because `fetch`/`superagent` both `require`
`wpapi.js`: sharing a chunk across entries breaks rolldown's CJS
`module.exports` codegen (it emits multiple sequential `module.exports =`
assignments into one file, so only the last one takes effect). Bundling
each entry's full dependency closure independently avoids that.
The browser UMD builds alias `node-fetch`/`form-data`/`fs` to thin stubs
in build/browser-shims/ that resolve to the native browser globals,
mirroring the old webpack `externals`/`node.fs` config; tsdown's default
auto-externalization of package.json dependencies is overridden for the
browser build so those aliases (and plain bundling of qs/li/superagent)
actually take effect.
.d.ts generation needs tsconfig.json's `module: "commonjs"` (the source
is plain CJS) and TypeScript's classic `tsc` generator. Two JSDoc
`@returns {WPRequest}` annotations in wpapi.js are widened to `{Object}`:
rolldown-plugin-dts can't bundle the cross-file CJS-shaped declaration
that annotation forces, and richer per-route typing is Phase 3's job
anyway.
Deletes webpack.config*.js, Gruntfile.js, and build/grunt/ (docs-only
Grunt tasks; Phase 6 replaces the whole docs pipeline).
vitest.config.js sets `globals: true` (Jest-compatible describe/it/expect
globals) and a setup file aliasing the `jest` global to `vi`, so the
~30 existing jest.fn()/jest.spyOn() call sites across the suite port
without any changes.
tests/unit/wpapi.js required the package root (`require('../../')`),
which resolves through package.json's `main` field. That field now
points at the tsdown-built dist/index.cjs instead of source wpapi.js,
so the WPRequest class that test imports directly no longer matched
the one used inside the WPAPI instance it was constructing, breaking
an `instanceof` assertion. Fixed by requiring `../../wpapi` directly,
which also decouples this suite from wherever `main` happens to point
(relevant again once Phase 2 rebinds `main` to the fetch-bound export).
eslint.config.js carries forward the exact same custom rules as the old .eslintrc.js (and its per-directory overrides for tests/, build/, and lib/data/), including the project's space-inside-parens/brackets/braces convention. Prettier is added (.prettierrc.json: tabs, single quotes) but deliberately NOT run across the existing codebase or wired into `lint`/CI: Prettier has no option to reproduce that bracket-spacing convention, and running it would fight the ESLint rules that enforce it on every file it touches. It's available via `npm run format`/`format:check` for new code or a future full-codebase decision. Deletes .eslintrc.js, .eslintignore, and the per-directory .eslintrc.js overrides, all superseded by the flat config.
Mechanical comma-dangle fixes across files not previously covered by the old eslint script's target list (it never linted fetch/ or superagent/ directly), applied via `eslint --fix`. ESLint 9's eslint:recommended also added two rules the old ESLint 4 didn't have, surfacing two pre-existing, harmless-but-worth-fixing issues: no-prototype-builtins (two `.hasOwnProperty()` calls, now `Object.prototype.hasOwnProperty.call(...)`) and no-unused-vars (one unused `catch (e)` binding, now an optional catch binding).
package.json: engines.node >=18, drop browserslist/IE11 support, drop the Jest config block (superseded by vitest.config.js). Scripts: jest -> vitest across the board; eslint invocation updated for the flat config; a `typecheck` script is added (`tsc --noEmit`; not yet meaningful since checkJs is off until Phase 3); `docs` no longer depends on the now-deleted Grunt (runs jsdoc directly); the `zip` script (grunt-zip against the old browser/ output) is dropped. Removes the corresponding webpack/Grunt/Babel/ESLint4/Jest devDependencies. package-lock.json is now committed. The old "consuming applications maintain their own lockfile" rationale doesn't hold once this repo has its own CI: actions/setup-node's `cache: npm` and `npm ci` both need one checked in for reproducible builds. Deletes .travis.yml, replaced by GitHub Actions.
ci.yml has three jobs, not a flat 18/20/latest matrix: the toolchain itself doesn't support that range (tsdown/rolldown need Node >=22.18, vitest needs >=20). `test` (Node 22/24/latest) runs lint, typecheck, build, and unit tests, and uploads dist/ as an artifact from the `latest` leg. `dist-smoke` (Node 18/20) downloads that artifact and just requires the pre-built CJS/ESM entries, checking the published `engines.node: >=18` promise against the actual output without needing the build tooling to run on old Node. `integration` (Node 22) boots @wordpress/env and runs test:integration; note `wp-env logs` defaults to `--watch: true` and never exits, so it's invoked directly with `--no-watch` rather than through the `env:logs` dev-convenience script. release.yml is scaffolded per the plan (build, test, `npm publish --provenance` on a GitHub Release) but inert until an NPM_TOKEN secret is added and publishing resumes.
Replaces webpack + Grunt + Babel with tsdown (dual ESM+CJS+UMD+.d.ts, generated exports map), Jest with Vitest, ESLint 4 with ESLint 9 flat config + Prettier scaffold, sets Node 18+ / drops IE11, and stands up GitHub Actions CI (ci.yml) with a release.yml scaffold, replacing .travis.yml. No library behavior changed. Full suite green against wp-env: 805 passed, 16 skipped, 0 failed — identical to the Phase 0 baseline. See handoff.md for the tricky decisions and workarounds this phase required.
Two suites written ahead of the implementation, per break-tests-first: - fetch/tests/unit/fetch-transport.js exercises the fetch transport's internals directly (the existing fetch/tests/unit/fetch.js only spies on the transport boundary, so the actual HTTP code had no unit coverage). It stubs the global fetch and asserts the rewritten transport will use it — with native FormData/Blob/File uploads covering all documented .file() input shapes: Blob, File, Buffer + name, and disk path (with and without a name override). - superagent/tests/unit/superagent.js replaces the old superagent-bound WPAPI suite: wpapi/superagent is being removed, and the subpath must throw a clear migration error pointing at the fetch-based default. All 19 tests fail against the current node-fetch/form-data implementation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The transport now calls the global fetch available in every supported runtime (Node 18+, browsers, workers) instead of requiring node-fetch, and media uploads build a native FormData: Blob/File attachments are appended as-is, Buffers are wrapped in a Blob, and string paths are read from disk into a Blob (with the on-disk basename as the default upload name, matching form-data's old behavior). The node:fs/promises require is lazy, inside the path branch, to keep Node built-ins out of browser bundles. .file() now also validates that a name accompanies any attachment that cannot carry its own (a Buffer, or a Blob that is not a File) — WordPress rejects uploads without an extension, so fail early and clearly instead. This makes the 19 transport-contract tests from the previous commit pass and drops the node-fetch, form-data and fs requires; the dependencies themselves are removed from package.json in a following commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rror The superagent transport directory is deleted, and superagent dropped from optionalDependencies. wpapi/superagent remains an exported subpath, but now throws an error explaining the removal and pointing at the fetch-based default export (and at custom transports for anyone who depended on superagent-specific behavior). Keeping the entry means upgrading consumers get that message rather than a bare ERR_PACKAGE_PATH_NOT_EXPORTED. Integration suites previously ran once per transport via describe.each; the superagent rows are dropped and the fetch rows kept. The wpapi-superagent browser UMD bundle is no longer built. build/scripts/update-default-routes-json.js was the last remaining superagent consumer; it now downloads the endpoint JSON with native fetch (same redirect-following, content-type validation and error-exit behavior). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
require('wpapi') / import WPAPI from 'wpapi' is batteries-included
again: the index entry now builds from fetch/index.js, restoring v1.x
ergonomics by reversing the 2.0.0-alpha's transport-less split.
wpapi/fetch remains as an alias subpath for code written against the
alpha. The transport-less base constructor is still available in-source
(wpapi.js + lib/bind-transport.js) but is no longer shipped as its own
entry.
With node-fetch and form-data gone from the transport, the browser UMD
build no longer needs shim aliasing — build/browser-shims/ is deleted
and the last optionalDependencies are dropped. The dist-smoke CI job now
also asserts the main entry has a bound transport and that the
superagent stub throws its migration error on Node 18/20.
Known tradeoff: dist/index.d.cts now carries the fetch entry's weaker
generated type (a bare callable) instead of wpapi.js's fuller shape.
Real typings are Phase 3's TypeScript migration.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The plugin registered its callback on 'wp enqueue scripts' (spaces), a hook that never fires, and read package.json relative to the undefined constant __FILE. Both typos meant the bundled convenience plugin never actually enqueued the script. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
README installation, upgrade, media-upload and custom-transport sections now describe the 2.0.0 behavior: native fetch built in, no superagent, Node 18+ floor, no IE11. The fetch/ subpath README no longer instructs installing isomorphic-unfetch, and the CHANGELOG's v2.0.0 entry reflects the final (non-alpha) breaking-change list. Full docs overhaul remains Phase 6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…safe globals Three review findings against the rewritten transport: - The unsupported-method guard (checkMethodSupport) lived only in the deleted superagent transport, so e.g. calling .delete() on a GET-only route would now transmit a live DELETE instead of throwing locally as v1 defaults did. All five verbs now check the route's supported methods before dispatch. - _httpHead resolved the response headers without checking response.ok, so .headers() on a missing resource resolved instead of rejecting. HEAD responses carry no body to extract an API error object from, so reject with the response itself. - global.Buffer / global.btoa throw ReferenceError in browsers (no `global` identifier outside Node; the old webpack build shimmed it, rolldown does not). Now globalThis, which is universal. Also collapses the FormData.append name branch: WebIDL treats an explicitly-undefined filename as omitted, so the two calls were equivalent (covered by the File-name-preservation test). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review findings against the new .file() validation: - The File-instanceof exemption used the File global, which only exists from Node 20 — on Node 18 (our documented floor) a named File object was misclassified as a bare Blob and rejected. A Blob attachment is now exempted from the name requirement by its own name property, which any File carries regardless of runtime. - Stream attachments (accepted by the old form-data/superagent stack, impossible with native FormData) previously fell through validation and produced an opaque undici error or a stringified '[object Object]' form field. .file() now rejects anything that is not a path string, Buffer, Blob or File with an explicit streams-are-gone message. - global.Buffer swapped for globalThis.Buffer, which does not throw as an undeclared identifier in browsers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two review findings against the dist output, fixed with a post-build pass (build/scripts/finalize-dist.js, now the last step of npm run build): - tsdown emitted the index and fetch entries as two independent, byte-identical ~95K bundles, so 'wpapi' and 'wpapi/fetch' resolved to two separate module instances (distinct constructors, instanceof failures across the boundary) and the package carried ~190 kB of duplication. fetch.* are now one-line re-exports of index.*. - The generated index.d.* declarations erased the export to an anonymous Function shape with no construct signature — worse than no types, since shipping `types` also disables any @types/wpapi fallback and `new WPAPI()` failed to compile. Typing the source directly hits rolldown-plugin-dts's cross-file CJS declaration crash (the known Phase 1 constraint), so the script writes an honest interim declaration instead: constructable, known statics, permissive instance surface. Verified with a strict-mode nodenext consumer compile. Phase 3's TypeScript migration deletes this workaround. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- node-wpapi.php enqueued browser/wpapi.min.js, a path nothing has ever built; with the hook-name typo fixed that would have become a live 404 on every page load. Point it at the UMD bundle tsdown actually emits (dist/browser/wpapi.umd.js) and drop the SCRIPT_DEBUG branch — there is only one browser artifact now. - update-default-routes-json.js attached its download-failure handler as a trailing .catch, so an exception thrown by the downstream simplify-and-write callback was misreported as "Could not download endpoint JSON". The handler now sits alongside the callback so downstream errors surface with their real stack. - The superagent test directory held a single stub assertion but kept four config references alive (vitest include, eslint files, two npm scripts). Moved the test to tests/unit/superagent-stub.js and dropped the extra config surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Also notes the accepted upload behavior delta in CHANGELOG/README: native Blobs carry no filename-inferred MIME type, so the multipart part is sent as application/octet-stream and WordPress resolves the real type from the file name (a typed Blob/File gives explicit control). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Native fetch/FormData/Blob transport, batteries-included main export, superagent removed (subpath stubs a migration error), node-wpapi.php fixes, and a review pass restoring v1 client-side guards. Suite: 599 unit + 110 integration passed, 8 skipped (discover, deferred to Phase 5). See handoff.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.