Uh oh!
There was an error while loading. Please reload this page.
Allow --module commonjs --moduleResolution bundler - #62320
Merged
Andrew Branch (andrewbranch) merged 1 commit intoAug 22, 2025
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR allows the combination of --module bundler with --moduleResolution commonjs, which was previously prohibited with a program error. This change supports users who transpile their TypeScript code to CommonJS before bundling, particularly those using tools like Webpack's ts-loader with CommonJS module compilation settings.
Key changes:
- Removes the restriction preventing
--moduleResolution bundlerfrom being used with--module commonjs - Updates error messages to include "commonjs" as a valid module option with bundler resolution
- Adds comprehensive test coverage for the new combination of settings
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/compiler/program.ts | Updates validation logic to allow --module commonjs with --moduleResolution bundler |
src/compiler/diagnosticMessages.json | Updates error message text to include "commonjs" as a valid option |
tests/cases/conformance/moduleResolution/bundler/bundlerCommonJS.ts | New test file demonstrating CommonJS module resolution with bundler |
tests/cases/conformance/moduleResolution/bundler/bundlerOptionsCompat.ts | Updates existing test to use "nodenext" instead of "commonjs" |
| Multiple baseline files | Updates test baselines to reflect the new allowed combination and updated error messages |
Uh oh!
There was an error while loading. Please reload this page.
Andrew Branch (andrewbranch)
requested review from
Jake Bailey (jakebailey) and Wesley Wigham (weswigham)August 22, 2025 17:58
Jake Bailey (jakebailey)
approved these changes
Aug 22, 2025
Comment on lines
+34
to
+35
| // @Filename: /real-imports.mts | ||
| import { x } from "pkg"; // Error |
MemberAuthor
There was a problem hiding this comment.
As of #57896 (TS 5.6), .mts and .cts extensions override the --module setting. Since this import will emit as an import, and the package has only a require entrypoint, the resolution fails.
Uh oh!
There was an error while loading. Please reload this page.
Andrew Branch (andrewbranch) added a commit
that referenced
this pull request
Oct 20, 2025
Andrew Branch (andrewbranch) added a commit
that referenced
this pull request
Oct 20, 2025
Co-authored-by: Andrew Branch <andrewbranch@users.noreply.github.com> Co-authored-by: Andrew Branch <andrew@wheream.io>
--module bundler --moduleResolution commonjs--module preserve --moduleResolution commonjs--module preserve --moduleResolution commonjs--module commonjs --moduleResolution bundlerKDR (kdr) pushed a commit
to cloudglue/cloudglue-js
that referenced
this pull request
Apr 21, 2026
…ation issues. (#111) # Summary This PR updates the js sdk to include the grain data connector. - Added three new filter options to the ListDataConnectorFilesParams - updated package.json to resolve depreciated legacy settings build issue. - Minor version bump to 0.7.8 ## Build issue [article](https://visualstudiomagazine.com/articles/2026/03/23/typescript-6-0-ships-as-final-javascript-based-release-clears-path-for-go-native-7-0.aspx#:~:text=Microsoft%20on%20March%2023%20released,in%20the%20Go%20programming%20language.) [ref](https://www.typescriptlang.org/tsconfig/#moduleResolution) to fix this in the narrowest way possible we have opted to follow this [advice](https://devblogs.microsoft.com/typescript/announcing-typescript-6-0/#combining---moduleresolution-bundler-with---module-commonjs) and this [advice](microsoft/TypeScript#62320) ## Testing - node - ensured sdk is importable and runnable on local node. ``` cd {common-js directory} run node const { Cloudglue } = require('./dist/src/index.js') const client = new Cloudglue({ apiKey: 'not my real api key'}) const myFiles = await client.files.listFiles({limit: 10}); console.log(myFiles) { object: 'list', data: [ { id: '****************', object: 'file', bytes: 148648709, ... ``` -Browser - setup simple vita app ensure we can install, import and use common-js main.ts ``` import { Cloudglue } from '@cloudglue/cloudglue-js'; // Smoke: constructor only (no network). Use a real key + API calls to test CORS. const _client = new Cloudglue({ apiKey: 'browser-smoke-not-a-real-key' }); console.info('Cloudglue client OK', _client.files != null); ``` in browser we see: Cloudglue client OK true - ensure filters are working - tested limit, meeting_type, from, to filters from sdk in the following way: ``` const client = new Cloudglue({ apiKey: "1234567890", baseUrl: "http://localhost:3002/v1"}); const dataConnectors = await client.dataConnectors.list(); const grain = dataConnectors.data[0]; const params = { limit: 10, }; const files = await client.dataConnectors.listFiles(grain.id, params); console.info('Cloudglue client OK', files); ``` and received successful responses. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for the Grain data connector with new filtering capabilities for connector file operations. * Introduced three new optional filters: title search, team, and meeting type. * **Documentation** * Updated data connector documentation to reflect Grain connector support and usage examples. * **Chores** * Version updated to 0.8.0. * Enhanced type safety and optimized build configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Merged
4 tasks
Aryan (aryan877) added a commit
to aryan877/credo
that referenced
this pull request
May 7, 2026
…olution The @CREDO scope was unowned on npm. Switch every reference to @credohq/sdk so the package can publish under a real org tied to credohq.xyz. Upgrade typescript to ^6.0.3 and switch tsconfig moduleResolution from the deprecated Node10 to bundler. PR microsoft/TypeScript#62320 specifically allows the module: CommonJS + moduleResolution: bundler combo for projects that publish CJS and need to drop the deprecation warning. ignoreDeprecations is no longer needed. Updates 14 TS/TSX import strings across credo-app, the three adapters, and credo-backend, plus the SDK and consumer package.json files. Adds publish metadata (repository, homepage, bugs, keywords) to the SDK package.json so the npmjs.com page renders properly. Package @credohq/sdk@0.1.0 is now live on npm.
kulrashm-jpg added a commit
to kulrashm-jpg/Omnivyra
that referenced
this pull request
Aug 9, 2026
Completes the TS7-migration D1 step. The Railway worker keeps CommonJS emit and its existing runtime; only the resolver the compiler uses to FIND types changes. tsconfig.worker.json moduleResolution: "node" (node10) -> "bundler" tsconfig.json remove the temporary `ignoreDeprecations: "6.0"` package-lock.json ts-jest 29.4.9 -> 29.4.12 (+ its nested semver 7.7.4 -> 7.8.5) `module: "commonjs"` is RETAINED. commonjs + bundler is an intentionally supported TypeScript 6 combination (microsoft/TypeScript#62320, milestone 6.0.0) and the release notes name it as the recommended upgrade path off the deprecated node10 resolver -- which is exactly this repo's situation. The ts-jest bump is required, not incidental: 29.4.9 unconditionally overrode moduleResolution to Node10 inside fixupCompilerOptionsForModuleKind(), so removing `ignoreDeprecations` while it was installed broke EVERY jest suite. 29.4.12 uses Node10 only as a fallback default, so the project's `bundler` setting flows through. `rootDir: "."` stays -- that one is permanent (TS6 no longer infers it; TS5011). Done via `npm update ts-jest`, so package.json is untouched: 29.4.12 already satisfies the declared `^29.1.2`. No other dependency moved. NOT done, deliberately: no Node16/NodeNext conversion, no ESM, no `.js` extensions added, no change to the ~952 dynamic-import sites, no package `type` change, no Node/Vercel/Railway/Dockerfile change. Zero source files changed. EMIT EQUIVALENCE (the load-bearing gate). Worker built twice with TS 6.0.3 into isolated output dirs -- node10 vs bundler -- with the real fix-worker-aliases.js run on both: 3768 JS files each, 185 alias rewrites across 135 files each, and `diff -r` BYTE-IDENTICAL with ZERO differing entries INCLUDING source maps. The ThreadSequencePreview.js exception carried since Phase C did not appear: that difference was TS5.9-vs-TS6 compiler drift, and both sides here are 6.0.3, so moduleResolution was the only variable and it changes no output at all. RUNTIME (isolated). Image built from Dockerfile.worker unmodified; tsc + alias rewrite inside the image reported the same 185/135. Ran on a dedicated docker network with its own Redis and the local cert Supabase -- never the production-connected omnivyra-worker, which was left running and untouched, and never production data (env scanned for prod markers first). /health 200, healthy 40+ minutes, 3 cron cycles, and 0 occurrences of MODULE_NOT_FOUND / ERR_MODULE_NOT_FOUND / ERR_REQUIRE_ESM / "Cannot find module" across the whole run. All 403 downlevelled dynamic-import sites in the shipped dist resolved from their own importer context: 403/403, 0 unresolved. 4 representative queue jobs (semantic-indexing, analytics-ingestion, lead-thread-recompute, intelligence-polling) all completed: 0 failed, 0 DLQ. Validation: typecheck:ci 3/3 clean (baseline 0, actual 0); worker typecheck exit 0; typecheck:certification 0/1 and 340/443 PASS -- unchanged from D0, so no net-new type debt. Jest under ts-jest 29.4.12 with no TS5107 and no TS5011. Production build exit 0, BUILD_ID kWP7QzUs9wCcnbbv6RGQV, all manifests present, 1321 server pages and 424 static chunks -- both exactly matching the D0 baseline. check:ssrf / check:authz / check:route-policy / check:migrations all pass. 42-suite regression: failing-suite SET identical to baseline (campaign_health, content_pipeline -- both pre-existing), 380 passed both runs. No baseline file and no test was modified. Co-Authored-By: Claude Opus 5 (1M context) <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 freeto 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.
Prerequisite for #62200
This has worked in a coherent way since
--module preservewas implemented, but was prohibited with a program error.--moduleResolution bundler(like every other modern TS resolution mode) resolves package.json"exports"conditions according to the output module syntax being emitted. So in--module commonjs, imports in .ts and .js files always resolve with the"require"condition set.This combination of settings is very rarely going to be what people actually want, and is mostly intended as a "my code works, don't bother me" upgrade path for users on
--module commonjs --moduleResolution: node. (More poignantly, it doesn't seem like deprecating--module commonjsis an option at this point, and with--moduleResolution node10going away, that leaves nomoduleResolutionsettings left that are legal withcommonjs.) However, it's also the correct settings for anyone transpiling their code to CommonJS before resolving imports through a bundler. This includes Webpack ts-loader users who have--module commonjsin their ts-loader tsconfig. However, you should not use this combination of settings just because you set your bundler's output to emit a CommonJS module, so our docs / blog post need to be intentional about highlighting this.