Skip to content

Fix commonjs/esm/types output #610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Fix commonjs/esm/types output #610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8c7f8c2
test: Rollup options - extract into its own directory
vio Aug 31, 2025
ce3836b
test: Add vite package test
vio Aug 31, 2025
62d4051
test: Rollup - add config tests
vio Aug 31, 2025
d05f670
test: Vite - add config tests
vio Aug 31, 2025
9fd5f54
build: Individual test jobs for each bundler
vio Aug 31, 2025
04fa477
build: Add vite job
vio Aug 31, 2025
fd6e4cc
fix: Export cjs/esm modules
vio Aug 31, 2025
125fa5f
chore: Bump rollup-plugin-stats to v1.5.1
vio Sep 2, 2025
42e546e
build: Update tsconfig
vio Sep 2, 2025
9f4b79d
DROP - release 2.1.5-beta.5
vio Sep 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 59 additions & 6 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ jobs:
with:
name: build-artifacts
retention-days: 1
path: dist*/
path: lib*/

test-package:
name: Test package on node ${{ matrix.node }} / ${{ matrix.os }} / rollup v${{ matrix.rollup }}
test-package-rollup:
name: "rollup v${{ matrix.rollup }} / node ${{ matrix.node }} / ${{ matrix.os }}"
needs:
- build
runs-on: ubuntu-latest
Copy link

@coderabbitai coderabbitai bot Sep 1, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use the matrix OS for the rollup job

Windows variants won’t run with runs-on hardcoded.

Apply:

-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
runs-on: ubuntu-latest
runs-on: ${{ matrix.os }}
🤖 Prompt for AI Agents 
In .github/workflows/ci.yml around line 58, the rollup job currently hardcodes
"runs-on: ubuntu-latest" which prevents Windows/macOS runs; change it to use the
job matrix by replacing runs-on: ubuntu-latest with runs-on: ${{ matrix.os }}
and add/ensure a strategy.matrix.os section for the job (e.g., include
windows-latest, ubuntu-latest, macos-latest or your supported OS list) so the
job runs across the matrix; adjust any OS-specific steps if needed.

Expand Down Expand Up @@ -99,16 +99,69 @@ jobs:
npm install
npm install rollup@${{ matrix.rollup }} --force
npx rollup -v
working-directory: test/package
working-directory: test/package/rollup

- name: Test package
run: npm run test:package
run: npm run test
working-directory: test/package/rollup

test-package-vite:
name: "vite v${{ matrix.vite }} / node ${{ matrix.node }} / ${{ matrix.os }}"
needs:
- build
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
node: ["22", "20", "18"]
os: [ubuntu-latest, windows-latest]
vite: ["5", "6", "7"]
exclude:
- os: windows-latest
node: 20
- os: windows-latest
node: 18
- vite: 7
node: 18
steps:
Comment on lines +108 to +126
Copy link

@coderabbitai coderabbitai bot Aug 31, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Matrix os is defined but not used; runs-on is hardcoded to ubuntu (vite job)

Same issue as rollup job; Windows variants won’t execute.

-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test-package-vite:
name: "vite v${{ matrix.vite }} / node ${{ matrix.node }} / ${{ matrix.os }}"
needs:
- build
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
node: ["22", "20", "18"]
os: [ubuntu-latest, windows-latest]
vite: ["5", "6", "7"]
exclude:
- os: windows-latest
node: 20
- os: windows-latest
node: 18
- vite: 7
node: 18
steps:
test-package-vite:
name: "vite v${{ matrix.vite }} / node ${{ matrix.node }} / ${{ matrix.os }}"
needs:
- build
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
node: ["22", "20", "18"]
os: [ubuntu-latest, windows-latest]
vite: ["5", "6", "7"]
exclude:
- os: windows-latest
node: 20
- os: windows-latest
node: 18
- vite: 7
node: 18
steps:
🤖 Prompt for AI Agents 
.github/workflows/ci.yml around lines 107 to 125: the test-package-vite job
defines a matrix.os but the job hardcodes runs-on: ubuntu-latest so Windows
variants never run; change runs-on to use the matrix value (runs-on: ${{
matrix.os }}), ensure the matrix.os contains the desired runners (ubuntu-latest,
windows-latest), and keep or adjust the existing exclude entries so the intended
combinations are still skipped.

- name: Checkout repo
uses: actions/[email protected]

- name: Setup node & npm
uses: relative-ci/.github/actions/[email protected]
with:
version: ${{ matrix.node }}
cache: never
install: false

- name: Install vitest
run: npm install vitest

- name: Download build files
uses: actions/[email protected]
with:
name: build-artifacts

- name: Setup local npm registry
run: ./scripts/setup-registry.sh

- name: Install test package dependencies
run: |
npm install
npm install vite@${{ matrix.vite }} --force
npx vite -v
working-directory: test/package/vite
Comment on lines +145 to +153
Copy link

@coderabbitai coderabbitai bot Sep 2, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Windows: force bash for the registry script and installs (Vite job) + trim trailing space

Prevents failures on windows and fixes YAML lint trailing spaces.

Apply:

       - name: Setup local npm registry
-        run: ./scripts/setup-registry.sh
+        run: ./scripts/setup-registry.sh
+        shell: bash

       - name: Install test package dependencies
         run: |
-          npm install 
+          npm install
           npm install vite@${{ matrix.vite }} --force
           npx vite -v
         working-directory: test/package/vite
+        shell: bash
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Setup local npm registry
run: ./scripts/setup-registry.sh
- name: Install test package dependencies
run: |
npm install
npm install vite@${{ matrix.vite }} --force
npx vite -v
working-directory: test/package/vite
- name: Setup local npm registry
run: ./scripts/setup-registry.sh
shell: bash
- name: Install test package dependencies
run: |
npm install
npm install vite@${{ matrix.vite }} --force
npx vite -v
working-directory: test/package/vite
shell: bash
🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 150-150: trailing spaces

(trailing-spaces)

🤖 Prompt for AI Agents 
.github/workflows/ci.yml lines 145-153: the workflow step runs a shell script
and multiline npm commands which can fail on Windows and contains a trailing
space after "npm install "; change the steps to force bash for the registry
script and the multiline install block (e.g., use "bash
./scripts/setup-registry.sh" and run the multiline commands under bash with
"bash -lc") and remove the trailing space after "npm install" so the YAML is
lint-clean and the commands run reliably on Windows.


- name: Test package
run: npm run test
working-directory: test/package/vite

tests:
needs:
- lint
- test-unit
- test-package
- test-package-rollup
- test-package-vite
runs-on: ubuntu-latest
steps:
- run: exit 0
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
*.log
.DS_Store
node_modules
dist
lib
.env
123 changes: 117 additions & 6 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 23 additions & 17 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "rollup-plugin-webpack-stats",
"version": "2.1.4",
"version": "2.1.5-beta.5",
"private": false,
"license": "MIT",
"keywords": [
Expand All @@ -11,19 +11,19 @@
"stats",
"bundle-stats"
],
"main": "dist/index.cjs",
"module": "dist/index.mjs",
"typings": "dist/index.d.ts",
"main": "lib/cjs/index.js",
"module": "lib/esm/index.js",
"typings": "lib/types/index.d.ts",
"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.mjs",
"require": "./dist/index.cjs"
"types": "./lib/types/index.d.ts",
"import": "./lib/esm/index.js",
"require": "./lib/cjs/index.js"
},
"./transform": {
"types": "./dist/transform.d.ts",
"import": "./dist/transform.mjs",
"require": "./dist/transform.cjs"
"types": "./lib/types/transform.d.ts",
"import": "./lib/esm/transform.js",
"require": "./lib/cjs/transform.js"
}
Comment on lines 18 to 27
Copy link

@coderabbitai coderabbitai bot Sep 1, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify declaration emit to lib/types

Ensure tsconfig sets declaration output once (to avoid duplicated emits) and to lib/types to satisfy exports/types.

Run:

Also applies to: 43-44, 49-51

🏁 Script executed:

#!/bin/bash
# Inspect tsconfigs for declaration settings
fd -a 'tsconfig*.json'
rg -nP '"declaration"\s*:\s*true' tsconfig*.json || true
rg -nP '"emitDeclarationOnly"\s*:\s*true' tsconfig*.json || true
rg -nP '"declarationDir"\s*:\s*"lib/types"' tsconfig*.json || true
# Show mapped export types
jq -r '.exports["."].types, .exports["./transform"].types, .types? // .typings?' package.json

Length of output: 618

tsconfig.base.json: configure declaration output directory
Add under compilerOptions in tsconfig.base.json:

"emitDeclarationOnly": true,
"declarationDir": "lib/types"

to emit all .d.ts files only into lib/types and prevent duplicate declaration files across your CJS/ESM builds.

🤖 Prompt for AI Agents 
In package.json around lines 18 to 27, the project is emitting duplicate .d.ts
files into both CJS and ESM build outputs; update tsconfig.base.json under
compilerOptions to set "emitDeclarationOnly": true and "declarationDir":
"lib/types" so TypeScript emits only declaration files into lib/types
(preventing duplicates across lib/cjs and lib/esm) and ensure any build scripts
reference that declarationDir for packaging.

Comment on lines +19 to 27
Copy link

@coderabbitai coderabbitai bot Sep 2, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify .d.ts emit goes to lib/types exactly once

Ensure tsconfig emits declarations only to lib/types to satisfy these exports and avoid duplicates across CJS/ESM.

Run:

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "Tsconfig files:" && fd -a 'tsconfig*.json' || true
echo
echo 'Declaration settings:'
rg -nP '"declaration"\s*:\s*true' tsconfig*.json || true
rg -nP '"emitDeclarationOnly"\s*:\s*true' tsconfig*.json || true
rg -nP '"declarationDir"\s*:\s*"lib/types"' tsconfig*.json || true
echo
echo 'Package types mapping:'
jq -r '.exports["."].types, .exports["./transform"].types, .types? // .typings?' package.json

Length of output: 882

Configure TypeScript to emit declarations only to lib/types
In your declaration-producing tsconfig (e.g. tsconfig.lib-esm.json or a shared base), add

"emitDeclarationOnly": true,
"declarationDir": "lib/types"

and turn off declaration in other build configs so .d.ts files aren’t duplicated under lib/esm or lib/cjs.

🤖 Prompt for AI Agents 
In package.json around lines 19 to 27, the package points to .d.ts files under
lib/types but your TypeScript build is currently producing declaration files
into multiple output folders; modify your declaration-producing tsconfig (e.g.
tsconfig.lib-esm.json or shared base) to set emitDeclarationOnly: true and
declarationDir: "lib/types", and ensure other tsconfig build configs have
declaration disabled so .d.ts files are not duplicated into lib/esm or lib/cjs
during the build.

},
"author": {
Expand All @@ -40,17 +40,20 @@
},
"homepage": "https://github.com/relative-ci/rollup-plugin-webpack-stats/blob/master/#readme",
"files": [
"dist"
"lib"
],
"engines": {
"node": ">=18"
},
"scripts": {
"prebuild": "rimraf ./dist",
"build": "tsc && rollup -c rollup.config.mjs",
"build": "npm run clean && rollup -c && npm run build-package-type",
"build-package-type": "echo '{\"type\":\"module\"}' > lib/esm/package.json",
"clean": "rimraf ./lib",
"lint": "exit 0",
"test:unit": "vitest test/unit",
"test:package": "cd test/package && vitest",
"test:package": "npm run test:package:rollup && npm run test:package:vite",
"test:package:rollup": "cd test/package/rollup && vitest",
"test:package:vite": "cd test/package/vite && vitest",
"bump": "./scripts/bump.sh",
"release": "./scripts/release.sh"
},
Expand All @@ -67,6 +70,8 @@
},
"devDependencies": {
"@release-it/conventional-changelog": "10.0.1",
"@rollup/plugin-commonjs": "28.0.6",
"@rollup/plugin-node-resolve": "16.0.1",
"@rollup/plugin-typescript": "12.1.4",
"@tsconfig/node18": "18.2.4",
"@types/node": "24.3.0",
Expand All @@ -76,17 +81,18 @@
"release-it": "19.0.4",
"rimraf": "6.0.1",
"rollup": "4.46.4",
"rollup-plugin-node-externals": "8.1.0",
"tslib": "2.8.1",
"typescript": "5.9.2",
"vitest": "3.2.4"
},
"dependencies": {
"rollup-plugin-stats": "1.5.0"
"rollup-plugin-stats": "1.5.1-beta.3"
},
"peerDependencies": {
"vite": "^5.0.0 || ^6.0.0 || ^7.0.0",
"rolldown": "^1.0.0-beta.0",
"rollup": "^3.0.0 || ^4.0.0",
"rolldown": "^1.0.0-beta.0"
"vite": "^5.0.0 || ^6.0.0 || ^7.0.0"
},
"peerDependenciesMeta": {
"vite": {
Expand Down
Loading
Loading