fix: repair broken type surfaces and ESM config entry before odysseus release#1827
fix: repair broken type surfaces and ESM config entry before odysseus release#1827bgub wants to merge 4 commits into
Conversation
| "module": "ESNext", | ||
| "moduleResolution": "node", |
There was a problem hiding this comment.
module: "ESNext" paired with moduleResolution: "node" is a mismatched combination — TypeScript 5.x expects moduleResolution to be "bundler", "node16", or "nodenext" when module is "ESNext". While this is typecheck-only (tsc is noEmit here and tsdown handles the real build), editors and CI runs may surface a TS5110 diagnostic on this combination. Using "bundler" would be the idiomatic match for ESNext output in a monorepo with a bundler-driven build.
| "module": "ESNext", | |
| "moduleResolution": "node", | |
| "module": "ESNext", | |
| "moduleResolution": "bundler", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/next/tsconfig.json
Line: 4-5
Comment:
`module: "ESNext"` paired with `moduleResolution: "node"` is a mismatched combination — TypeScript 5.x expects `moduleResolution` to be `"bundler"`, `"node16"`, or `"nodenext"` when `module` is `"ESNext"`. While this is typecheck-only (`tsc` is noEmit here and tsdown handles the real build), editors and CI runs may surface a TS5110 diagnostic on this combination. Using `"bundler"` would be the idiomatic match for ESNext output in a monorepo with a bundler-driven build.
```suggestion
"module": "ESNext",
"moduleResolution": "bundler",
```
How can I resolve this? If you propose a fix, please make it concise.|
Pushed a follow-up commit: widened |
TL;DR
Fixes three release blockers found while auditing the odysseus public API surfaces: gt-react ships broken type declarations,
gt-next/configcrashes at import time from ESM configs, and gt-i18n's shipped declarations reference an undeclared type.Code Changes
gt-react — broken types entry
src/index.types.tsre-exportedinitializeGTfrom@generaltranslation/react-core/pure, which no longer exports it (removed in feat!: clean up @generaltranslation/react-core public API surface #1815; the runtime entries were updated but the shared types entry was missed).pnpm --filter gt-react typecheckfailed with TS2305, consumers withskipLibCheck: falsegot a hard error on anygt-reactimport, and everyone else gotanyforinitializeGT. Now aliases the localinitializeGTSRAwrapper, matchingindex.server.ts.gt-next — ESM config build unusable
dist/config.mjsexecuted barerequire.resolve/requireat module-evaluation time (version probing) and__dirname(SWC wasm path), none of which exist in plain-Node ESM. Anynext.config.mjs/"type": "module"project crashed onimport 'gt-next/config'; the SWC compiler silently downgraded to'none'. All in-repo test apps usenext.config.ts(transpiled to CJS), which is why nothing caught it.src/plugin/nodeCompat.tsexporting a CJS/ESM-safenodeRequireanddistDir, used bygetStableNextVersionInfo.tsandconfig.ts(including the lazyrequire('@generaltranslation/compiler')in the webpack hook).polyfillRequire: falsestays as-is — the compat module is only imported from the config graph, which is never client-bundled.tsconfig.json:module: CommonJS→ESNext(typecheck-only; tsdown does the real transpile). Required forimport.meta, and aligns with every other package in the repo — gt-next was the lone outlier.gt-i18n — dangling type in shipped declarations
dist/internal.d.mtsdeclaredinterpolateMessage(... options: InterpolationOptions ...)but never declared or importedInterpolationOptions: the alias was tagged@internalandstripInternal: trueremoved the declaration while keeping the reference. Since gt-react's and gt-next's own d.ts import fromgt-i18n/internal, any app compiling withskipLibCheck: falsebroke just by importing them.translation-functions/types/options.ts(next toNormalizedLookupOptions, which it wraps) and dropped the@internaltag. The emitted declarations now import it from the shared options chunk, and it's nameable fromgt-i18n/internal/typesvia the existingexport type *.Patch changesets added for
gt-react,gt-next, andgt-i18n.Notes / Flags
initializeGT) typechecks. Plain-Node smoke tests:await import('gt-next/config')(ESM) andrequire('gt-next/config')(CJS) both load;nodeCompatresolvesdistDirto<pkg>/distandnext/package.jsonin both formats.validateCompilerdowngrades before wasm resolution. ThedistDirresolution itself is verified in both formats.module: ESNexttsconfig flip is the one change with wider blast radius — worth a second look, though tsc is noEmit-only for this package.typecheck, and tsdown's dts emit passes unresolved re-exports through. Recommend addingturbo typecheckand a plain-Node ESM import smoke test to the release gate (not included in this PR).Greptile Summary
This PR fixes three release blockers discovered while auditing the odysseus public API surfaces before release: a broken
initializeGTre-export ingt-react's shared types entry, an ESM crash onimport 'gt-next/config'caused by barerequire/__dirnameusage in the.mjsbuild, and a danglingInterpolationOptionsreference ingt-i18n's shipped declarations.index.types.ts): Drops the removed@generaltranslation/react-core/purere-export ofinitializeGTand aliases the localinitializeGTSRAwrapper instead, matchingindex.server.tsand restoring correct types for all consumers.nodeCompat.ts,config.ts,getStableNextVersionInfo.ts,tsconfig.json): Introduces a CJS/ESM-safenodeRequire/distDircompat helper; replaces all barerequire,require.resolve, and__dirnameuses in the config graph with it. The tsconfigmodulesetting is updated fromCommonJStoESNextto allowimport.metato typecheck — note thatmoduleResolutionremains"node", which is a mismatched combination formodule: ESNextand may produce TypeScript diagnostics (see inline comment).options.ts,interpolateMessage.ts): PromotesInterpolationOptionsfrom an@internal-tagged alias (stripped from.d.mtsbystripInternal) to a proper public export in the shared options module, resolving the dangling type reference in shipped declarations.Confidence Score: 4/5
The three targeted fixes are correct and well-scoped; the
moduleResolution: "node"+module: "ESNext"mismatch in the next package tsconfig is the only thing worth a follow-up.All three bug fixes are mechanically sound: the
initializeGTalias points at the right local wrapper,nodeRequire/distDircorrectly handle both module formats at runtime, andInterpolationOptionsis now properly declared in the shared options module. The one loose end is thetsconfig.jsonmodule/moduleResolution pairing —"ESNext"+"node"is a known mismatched combination in TypeScript 5.x that can surface diagnostics in editors and strict CI configurations, even though it doesn't block the build today.packages/next/tsconfig.json — the
module/moduleResolutionmismatch is worth revisiting before the next typecheck-in-CI effort described in the PR notes.Important Files Changed
nodeRequireanddistDir; runtime branching is correct andimport.meta.urlis only evaluated in the ESM path.modulefromCommonJStoESNextto allowimport.metato typecheck;moduleResolutionstays"node", which is a mismatched combination for ESNext — harmless here (tsdown builds, tsc is noEmit-only) but TypeScript may emit diagnostics about it.__dirnamewithdistDirfor wasm path resolution andrequire(...)withnodeRequire(...)in the lazy webpack hook; both substitutions are correct and preserve original semantics.require/require.resolvecalls withnodeRequire/nodeRequire.resolvethroughout; semantics unchanged, ESM-safe.initializeGTfrom@generaltranslation/react-core/pure(removed upstream) and aliases the localinitializeGTSRAinstead, matchingindex.server.ts.InterpolationOptionsas a named public export (alias forNormalizedLookupOptions<StringFormat>), fixing the dangling declaration in shipped.d.mtsfiles.@internal-taggedInterpolationOptionsalias and its associatedStringFormatimport, importing the type from the shared options module instead.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["next.config.mjs / next.config.ts"] -->|"import 'gt-next/config'"| B["dist/config.mjs OR dist/config.js"] B --> C["nodeCompat.ts"] C -->|"typeof require === 'function'"| D{"CJS or ESM?"} D -->|"CJS (dist/config.js)"| E["use native require"] D -->|"ESM (dist/config.mjs)"| F["createRequire(import.meta.url)"] E --> G["nodeRequire"] F --> G G --> H["getStableNextVersionInfo.ts\n(version probing)"] G --> I["lazy require('@generaltranslation/compiler')\n(webpack hook)"] C -->|"path.resolve(nodeCompatDirname, '..')"| J["distDir = dist/"] J --> K["path.resolve(distDir, './gt_swc_plugin.wasm')"]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A["next.config.mjs / next.config.ts"] -->|"import 'gt-next/config'"| B["dist/config.mjs OR dist/config.js"] B --> C["nodeCompat.ts"] C -->|"typeof require === 'function'"| D{"CJS or ESM?"} D -->|"CJS (dist/config.js)"| E["use native require"] D -->|"ESM (dist/config.mjs)"| F["createRequire(import.meta.url)"] E --> G["nodeRequire"] F --> G G --> H["getStableNextVersionInfo.ts\n(version probing)"] G --> I["lazy require('@generaltranslation/compiler')\n(webpack hook)"] C -->|"path.resolve(nodeCompatDirname, '..')"| J["distDir = dist/"] J --> K["path.resolve(distDir, './gt_swc_plugin.wasm')"]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(i18n): declare InterpolationOptions ..." | Re-trigger Greptile