feat(types): harden package shape (SD-2978)#3251
Merged
Merged
Conversation
The original SD-2978 commit only added consumer matrix + package-shape gates to release-superdoc.yml. Between sessions, release-stable.yml landed as the new central orchestrator for the npm `latest` publish lane, and SuperDoc's stable releases now route through that workflow instead of release-superdoc.yml (which is now `@next` only). Without this patch the stable lane would publish without the matrix or publint/attw gates running, defeating the purpose of "package-shape honest in CI" because the most-consumed dist-tag would still be unverified at publish time. Adds the same three steps (matrix, deep-type-audit, package-shape-gate) between Build packages and the orchestrator step in release-stable.yml.
…} (SD-2978)
The deep audit assumed `entry.types` is always a string. SD-2978's
manifest changes nest it as `{ import: '...d.ts', require: '...d.cts' }`
for the three entries that publish CJS. The audit threw
ERR_INVALID_ARG_TYPE on `path.resolve(root, entry.types)` when entry.types
was an object.
Add a small helper that picks the ESM target from either shape (string
or condition object). Walking the .d.ts side is sufficient because the
.d.cts is a generated shim of the same surface.
Verified: audit now exits 0 with the same 1799 findings as pre-fix.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e393bccef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…:es (SD-2978) Code review found that `pnpm run pack` was broken by the new prepack/postpack lifecycle. pnpm wraps prepack/postpack around scripts named exactly `pack`, and the user `pack` script itself invokes `pnpm pack` which triggers a second prepack. The inner prepack hit the "backup exists" guard and exited 1, the outer postpack was skipped, and the workspace was left with `package.json` mutated and `.package.json.prepack-backup` orphaned. Two changes: 1. Make `prepare` re-entrant. If the backup file exists AND the current manifest already looks sanitized (no `source` conditions, no `unpkg` or `jsdelivr` fields), no-op so the inner prepack falls through and the inner postpack can restore cleanly. If the backup exists but the manifest is NOT sanitized, fail loudly with a clear message — that means the workspace is in an inconsistent state from a previous failed pack and the developer needs to clean up. `restore` was already idempotent (no-op when backup missing). 2. Route `pack:local` through `pack:es` directly. Both ultimately do the same thing, but going through `pack:es` (whose name does not collide with the lifecycle trigger) avoids the double-fire on the common local-pack path. Verified with a synthetic harness covering 5 cases: clean run, double-fire (outer + inner), failed run (state inspection), retry after failure (self-heal), inconsistent state (loud refusal). All pass. Verified live in the worktree: - pnpm run pack:es: tarball created, manifest restored, no orphan backup - pnpm run pack: same (was broken before this commit, now works)
Contributor
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.122 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.79 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in vscode-ext v2.3.0-next.124 |
Contributor
|
🎉 This PR is included in superdoc-cli v0.8.0-next.96 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc-sdk v1.8.0-next.78 |
Contributor
|
🎉 This PR is included in superdoc v1.30.0-next.78 The release is available on GitHub release |
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
2 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.
Make SuperDoc's package shape honest so CommonJS+TypeScript consumers stop hitting TS1471/TS1541 and so the manifest matches what the tarball actually ships.
Six things change:
.d.ctsdeclaration shims for the three entries that advertiserequire/main(.,./types,./super-editor). Generated byscripts/ensure-types.cjsfrom the ESM.d.tsusing the TypeScript compiler API pluswith { "resolution-mode": "import" }import attributes.exports[*].typesbecomes nested{ import, require }for those three entries.prepack/postpacksanitize step that strips thesourcecondition from the publishedpackage.json(the source manifest keeps it for workspace dev, sincetsconfig.base.jsonresolves throughcustomConditions: ["source"]).unpkgandjsdelivrmanifest fields. They pointed atdist/superdoc.min.js, which the build doesn't emit../super-editor's conditions sotypescomes first, per publint.tests/consumer-typecheck/package-shape-gate.mjsthat runspublint --strictand@arethetypeswrong/cli --packagainst the packed tarball. Wired into PR CI,release-superdoc.yml, andrelease-stable.yml. The stable lane is what publishes the npmlatesttag, so gating onlyrelease-superdoc.ymlwould have left the most-consumed dist-tag unverified.tests/consumer-typecheck/src/imports-cjs.ctswithAssertNotAny<T>checks forrequire('superdoc'),require('superdoc/types'),require('superdoc/super-editor')undernode16/nodenext.The audit needed a small follow-up to read the new nested
types: { import, require }shape (it previously assumed string).Behavior change for bare CDN URLs. Removing the
unpkg/jsdelivrmanifest fields means bare URLs likehttps://cdn.jsdelivr.net/npm/superdoc/(no file path) now fall back tomain(the CommonJS bundle) instead of pointing atsuperdoc.min.js. Direct CDN URLs that include the full file path (the form AGENTS.md documents and thatcdn-smoke-testexercises) continue to work. The previous fields pointed at a path thepack:esbuild doesn't reliably emit, so the change brings the manifest into honest agreement with what ships.Note for consumers on TypeScript <5.3: the
with { "resolution-mode": "import" }import-attribute syntax used by the generated.d.ctsshims requires TS 5.3+ (Nov 2023). Anything older won't parse the shim.Verified:
node tests/consumer-typecheck/typecheck-matrix.mjs: 57 passed, 0 failednode tests/consumer-typecheck/package-shape-gate.mjs: publint clean, attw all 🟢 (CJS, ESM, node10, bundler) for all 3 CJS entriesnode tests/consumer-typecheck/deep-type-audit.mjs: 1799 findings, exit 0pnpm pack(backup file gone, source conditions intact)Linear: SD-2978