Allow `--module commonjs --moduleResolution bundler` by andrewbranch · Pull Request #62320 · microsoft/TypeScript · GitHub
Skip to content

Allow --module commonjs --moduleResolution bundler#62320

Merged
andrewbranch merged 1 commit intomicrosoft:mainfrom
andrewbranch:bundler-commonjs
Aug 22, 2025
Merged

Allow --module commonjs --moduleResolution bundler#62320
andrewbranch merged 1 commit intomicrosoft:mainfrom
andrewbranch:bundler-commonjs

Conversation

@andrewbranch
Copy link
Copy Markdown
Member

Prerequisite for #62200

This has worked in a coherent way since --module preserve was 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 commonjs is an option at this point, and with --moduleResolution node10 going away, that leaves no moduleResolution settings left that are legal with commonjs.) 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 commonjs in 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.

Copilot AI review requested due to automatic review settings August 22, 2025 17:54
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Aug 22, 2025
@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Aug 22, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 bundler from 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

Comment thread tests/cases/conformance/moduleResolution/bundler/bundlerCommonJS.ts
@github-project-automation github-project-automation Bot moved this from Not started to Needs merge in PR Backlog Aug 22, 2025
Comment on lines +34 to +35
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@andrewbranch andrewbranch merged commit e635bb9 into microsoft:main Aug 22, 2025
33 checks passed
@github-project-automation github-project-automation Bot moved this from Needs merge to Done in PR Backlog Aug 22, 2025
@andrewbranch andrewbranch deleted the bundler-commonjs branch August 22, 2025 21:18
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 6.0 milestone Aug 25, 2025
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>
@DanielRosenwasser DanielRosenwasser changed the title Allow --module bundler --moduleResolution commonjs Allow --module preserve --moduleResolution commonjs Feb 6, 2026
@DanielRosenwasser DanielRosenwasser changed the title Allow --module preserve --moduleResolution commonjs Allow --module commonjs --moduleResolution bundler Feb 6, 2026
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 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants