chore(ts): moduleResolution node -> nodenext (CJS) / bundler (ESM) - #1834
Open
pyramation wants to merge 4 commits into
Open
pyramation wants to merge 4 commits into
pyramation wants to merge 4 commits into
Conversation
- root tsconfig: module/moduleResolution nodenext (files stay CJS, no type:module) - every tsconfig.esm.json: moduleResolution bundler (dist/esm is bundler ESM) - codegen generate.ts: drop redundant dynamic imports of already-imported modules - embedder (template + generated sdk): use named OllamaClient export - presigned-url-plugin: narrow getTypeByName result to an output type - pgpm/core: ambient types for parse-package-name's untyped import entry - graphile-llm: exclude tests from the build like sibling packages
lerna version's syncWorkspaceLock step resolves bumped versions against the registry; a literal ^0.4.1 range on a workspace package fails there with ERR_PNPM_NO_MATCHING_VERSION as soon as coerce is bumped.
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review complete. No issues found — approved ✅. This PR introduces dual ESM builds by adding a
Candidate findings surfaced during review were checked against the actual source and did not hold up, so no actionable issues remain. Reviewed commit: 71a4b78 |
This was referenced Sep 15, 2026
Under nodenext tsc preserves import() in CommonJS output instead of downleveling it to require(), which Jest's CJS runtime cannot execute. pg and graphile-realtime-subscriptions are declared dependencies, so import them statically; site-deploy loads fs/promises and path through process.getBuiltinModule so it stays browser-safe.
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
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.
Summary
Same convention as constructive-io/constructive-db#3764 (tracked in constructive-planning#2056): retire
moduleResolution: node(deprecated in TS 6, removed in TS 7) without changing what we emit or how we publish.tsconfig.json:module: commonjs/moduleResolution: node->nodenext/nodenext. No package setstype: module, so nodenext classifies every.tsas CJS anddist/is byte-identical CommonJS.tsconfig.esm.json(106 files):+ "moduleResolution": "bundler"next to the existingmodule: es2022, describing whatdist/esmis (bundler ESM consumed via themodulefield). Noexportsmap, no.mjs, notype: module— dist-folder publishing untouched.Things
nodenext's stricter resolution surfaced and how they were fixed:graphile-presigned-url-plugin:build.getTypeByName('JSON')is aGraphQLNamedType(may be an input type); real bug hidden bynoderesolution. NowisOutputType(t) ? t : null.graphql/codegen/generate.ts: dropped threeawait import('./codegen/cli')/import('./output')calls whose targets were already statically imported in the same file (under nodenext a CJSimport()is preserved, not downleveled, and needs an extension). Test-fileimport()s are untouched: ts-jest forcesmodule: commonjs, so they still downlevel torequire(verified, suites pass).@agentic-kit/ollamaresolved through itsimportentry has no default export;embedder.ts(sdk/constructive-cli) and the codegen template that generates it now usem.OllamaClient.parse-package-name@1.0.0ships types only for itsrequirecondition; added an ambientdeclare moduleinpgpm/core/src/types/.graphile-llmwas compiling its__tests__intodist/; excluded like sibling packages.await import()of hard deps / Node builtins in CJS packages is now preserved by tsc (no longer downleveled torequire), and Jest's CJS runtime cannot execute it (A dynamic import callback was invoked without --experimental-vm-modules). This is what brokesite-deploy-ssg,graphile-search(BM25 index discovery silently skipped) and the uploads preset tests on the first CI run.pgingraphile-search/bm25-codec.tsandgraphile-realtime-subscriptionsingraphile-cache/create-instance.tsare declared dependencies, so they're plain static imports now.site-deploy/walk.tsmust stay browser-safe, so it loadsfs/promises/pathviaprocess.getBuiltinModule(Node >= 22.3,enginesis>=22), which works from CJS and ESM and is simply absent in a browser. Left alone on purpose:agentic/cli(ESM-onlypi),db-tools/pg-fixups.ts(best-effort,pgundeclared),codegen/output/writer.ts(oxfmtis ESM-only, optional) and the ollama embedders (lazy by design, no CJS test crosses them).agentic/pi-hostdepended on@constructive-io/coercevia literal^0.4.1(the only non-workspace:intra-repo dep).lerna version'ssyncWorkspaceLockstep then runspnpm install --lockfile-onlyand fails withERR_PNPM_NO_MATCHING_VERSIONfor the bumped^0.4.2. Switched toworkspace:^; lockfile updated accordingly.Verification on this branch (CI fully green):
pnpm buildclean (0 TS errors),pnpm lint0 errors,graphql/codegen21 suites / 364 tests +graphile-presigned-url-plugin24 tests pass,lerna version --conventional-commits --no-push --no-git-tag-versionsucceeds (127 packages), andpnpm -r publish --dry-run --no-git-checks(what lerna publish delegates to) packs all 121 public packages fromdist/withworkspace:^rewritten to concrete ranges. Sampledist/package.jsonstill hasmain/module/typesand noexports;require('./dist')loads.Follow-up (separate, mechanical):
rewriteRelativeImportExtensionswith.tssource imports + makage emittingdist/esm/package.json {"type":"module"}sodist/esmis also Node-loadable.Link to Devin session: https://app.devin.ai/sessions/d34558e831ad4bbf986f64cc4bd509bd
Open in Devin Desktop: https://app.devin.ai/desktop/session/d34558e831ad4bbf986f64cc4bd509bd?variant=devin
Requested by: @pyramation