fix: support Jest 28 CommonJS package imports - #2511
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Castiron custom code✅ No new custom-code files detected. 32 mixed files remain; 0 existing customizations changed. Compared 32 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 33142596705 --repo openai/openai-node \
--name castiron-custom-code-33142596705-1 --dir /tmp/castiron-custom-code-33142596705-1
git apply --stat /tmp/castiron-custom-code-33142596705-1/custom-code.patch
cat /tmp/castiron-custom-code-33142596705-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 222f3d7dffd9fd3d3d183473e1a7810e5e027d7e d3ccaa0cd32022c23875a9da4b1b87ae577fde3f
python3 scripts/castiron/custom_code_report.py report \
--base 222f3d7dffd9fd3d3d183473e1a7810e5e027d7e \
--head d3ccaa0cd32022c23875a9da4b1b87ae577fde3f --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-d3ccaa0cd320
cat /tmp/castiron-custom-code-d3ccaa0cd320/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed the exact current head. The postprocessor rewrites only the private X.509 state import for emitted CJS/ESM files, preserves declaration processing, and the four CommonJS/Jest ecosystem fixtures exercise both node and browser export conditions. No blocking findings. (GitHub does not permit this account to formally approve its own PR.)
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Reviewed d3ccaa0. No substantive issues found. The CommonJS rewrite targets the same guarded private state module already selected by the package import map and shared with the ESM wrapper, preserving X.509 registry identity across module formats. Declaration postprocessing and the optional Node-only transport boundary remain unchanged.
2 participants
Problem
After #2507 fixes the CommonJS + browser-condition Jest failure, the main-only examples job still has two CommonJS fixtures that fail under Jest 28:
Jest 28 does not resolve the package
importsalias from the emitted CommonJS modules.Fix
require('#x509-transport-state')calls to relative.jsstate-module paths during package postprocessing.Verification
Passed locally:
pnpm tsn ecosystem-tests/cli.ts node-ts4.5-jest28 --verbose --retry=0(failed before the fix, passed after)pnpm tsn ecosystem-tests/cli.ts node-ts-cjs node-ts-cjs-web node-ts-cjs-auto node-ts4.5-jest28 --verbose --parallel --jobs=4 --retry=0./scripts/test tests/ecosystem-cli.test.ts tests/ecosystem-browser-credential-security.test.ts(26 tests)node --experimental-strip-types scripts/test-packed-package.tspnpm lintpnpm exec tscAlso attempted the complete credential-free ecosystem matrix. All relevant Node/CommonJS fixtures passed; the local run could not complete Bun (binary unavailable), Deno (sandboxed cache permissions), or browser-direct/webpack checks (Chrome launch unavailable). These are local environment limitations, not failures in the changed paths.
Follow-up to #2507 and #2509.