Skip to content

replace commmonjs2 to module for export#88

Open
juah255 wants to merge 1 commit intomainfrom
fix/export-process
Open

replace commmonjs2 to module for export#88
juah255 wants to merge 1 commit intomainfrom
fix/export-process

Conversation

@juah255
Copy link
Copy Markdown
Contributor

@juah255 juah255 commented Apr 24, 2026

Summary by CodeRabbit

  • Chores
    • Updated package entry points from CommonJS to ES module format (.mjs), ensuring consistent module resolution for both ESM and CommonJS consumers.
    • Migrated webpack build configuration to output ES modules instead of CommonJS.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

📝 Walkthrough

Walkthrough

The pull request switches the package's build output from CommonJS to ES modules. This includes updating entry points in package.json to reference .mjs files and configuring webpack to emit ES module bundles instead of CommonJS format.

Changes

Cohort / File(s) Summary
Package Entry Points
package.json
Updated main entry point paths from dist/index.js to dist/index.mjs for main, module, and exports["."] conditions to align ESM and CommonJS consumers to the .mjs build.
Build Configuration
webpack.config.js
Converted webpack output format from CommonJS to ES modules by enabling experiments.outputModule, setting output.library.type to module, configuring chunkFormat: 'module', and extending output.environment to support module and dynamic import semantics.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 From CommonJS chains, we hop and leap,
To ES modules, smooth and sweet,
The .mjs files now take the stage,
A modern build for a modern age!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'replace commmonjs2 to module for export' accurately describes the main change: switching the webpack build and package.json entry points from CommonJS to ES module output.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/export-process

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Around line 14-15: The package.json currently points both "import" and
"require" to "./dist/index.mjs", which causes ERR_REQUIRE_ESM for CommonJS
consumers; fix by either making the package ESM-only (remove the "require" field
and add "type": "module" so consumers use the "import" entry) or produce a
dual-format build and repoint "require" to a CJS bundle (e.g.,
"./dist/index.cjs") while keeping "import": "./dist/index.mjs"; update your
webpack output and package.json fields ("import", "require") accordingly.

In `@webpack.config.js`:
- Around line 7-22: The webpack build only defines a single entry
("entry.index") and emits dist/index.mjs, but package.json declares subpath
exports like "./components/ui", "./providers", "./themes", "./utils", and
"./settings" that expect dist/<path>/index.js; add matching multi-entry outputs
in webpack.config.js (e.g., add entries for components/ui, providers, themes,
utils, settings under the entry object) or remove/update the subpath exports in
package.json; ensure emitted filenames/library.module format matches the exports
(update filename pattern and/or change export targets to .mjs) and keep the
experiments.outputModule and output.module settings intact so each subpath
produces its own dist/<subpath>/index(.mjs) file for proper resolution.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 060983de-07f9-4eb0-9c37-4ff455aef071

📥 Commits

Reviewing files that changed from the base of the PR and between 65d48db and eb741fa.

📒 Files selected for processing (2)
  • package.json
  • webpack.config.js

Comment thread package.json
Comment on lines +14 to +15
"import": "./dist/index.mjs",
"require": "./dist/index.mjs"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

Is require(esm) stable by default on Node.js 22.12 and what is the behavior on Node 22.0-22.11 when require() targets a .mjs file?

💡 Result:

No, require(esm) is not stable by default on Node.js 22.12. It is enabled by default (unflagged) but remains experimental, emitting an experimental warning on first use (unless from node_modules). It became fully stable later, as noted in end-of-2025 sources. On Node.js 22.0-22.11, require targeting a .mjs file throws ERR_REQUIRE_ESM by default. The feature requires the --experimental-require-module flag to load synchronous ESM (no top-level await), returning a namespace object similar to dynamic import. Without the flag, .mjs files cannot be required from CommonJS.

Citations:


"require": "./dist/index.mjs" is a semantic mismatch and breaks on Node 22.0–22.11.

.mjs files are always parsed as ES modules by Node.js, so pointing the require condition at an .mjs is incorrect. On Node 22.0–22.11 (within your declared engines.node: ">=22" range), this will throw ERR_REQUIRE_ESM for any CommonJS consumer. Even on Node 22.12 and later, require(esm) remains experimental (enabled-by-default but with warnings), not a stable feature.

Pick one of the following:

  1. ESM-only package (recommended, matches the webpack output): drop the require condition and declare the package as ESM.

    📦 Proposed change — ESM-only
       "name": "@wedevs/plugin-ui",
       "version": "2.0.0",
    +  "type": "module",
       "description": "Scoped, themeable UI components for WordPress plugins - ShadCN style",
       "main": "dist/index.mjs",
    @@
         ".": {
           "types": "./dist/index.d.ts",
    -      "import": "./dist/index.mjs",
    -      "require": "./dist/index.mjs"
    +      "import": "./dist/index.mjs"
         },
  2. Dual-format: emit a separate CJS bundle from webpack and repoint require at it (e.g. dist/index.cjs).

📝 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
"import": "./dist/index.mjs",
"require": "./dist/index.mjs"
"import": "./dist/index.mjs"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 14 - 15, The package.json currently points both
"import" and "require" to "./dist/index.mjs", which causes ERR_REQUIRE_ESM for
CommonJS consumers; fix by either making the package ESM-only (remove the
"require" field and add "type": "module" so consumers use the "import" entry) or
produce a dual-format build and repoint "require" to a CJS bundle (e.g.,
"./dist/index.cjs") while keeping "import": "./dist/index.mjs"; update your
webpack output and package.json fields ("import", "require") accordingly.

Comment thread webpack.config.js
Comment on lines 7 to +22
entry: {
index: './src/index.ts',
},
experiments: {
...( defaultConfig.experiments || {} ),
outputModule: true,
},
output: {
...defaultConfig.output,
path: path.resolve( __dirname, 'dist' ),
filename: '[name].js',
filename: '[name].mjs',
library: {
type: 'commonjs2',
type: 'module',
},
module: true,
chunkFormat: 'module',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Single index entry won't satisfy the subpath exports declared in package.json.

Webpack is configured with a single entry (index) emitting only dist/index.mjs, but package.json declares subpath exports for ./components/ui, ./providers, ./themes, ./utils, and ./settings that reference dist/<path>/index.js. Those files will never be produced by this build, so any consumer doing import ... from '@wedevs/plugin-ui/settings' (the documented usage pattern) will fail module resolution.

Either add corresponding entries here (and update the subpath exports to .mjs) or drop the subpath exports from package.json if only the root entry is supported.

🛠️ Example: multi-entry output
     entry: {
         index: './src/index.ts',
+        'components/ui/index': './src/components/ui/index.ts',
+        'providers/index': './src/providers/index.ts',
+        'themes/index': './src/themes/index.ts',
+        'utils/index': './src/utils/index.ts',
+        'components/settings/index': './src/components/settings/index.ts',
     },

Based on learnings: "Use sub-path exports for organized imports: wedevs/plugin-ui/settings, wedevs/plugin-ui/components/ui, wedevs/plugin-ui/providers, wedevs/plugin-ui/themes, wedevs/plugin-ui/utils".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@webpack.config.js` around lines 7 - 22, The webpack build only defines a
single entry ("entry.index") and emits dist/index.mjs, but package.json declares
subpath exports like "./components/ui", "./providers", "./themes", "./utils",
and "./settings" that expect dist/<path>/index.js; add matching multi-entry
outputs in webpack.config.js (e.g., add entries for components/ui, providers,
themes, utils, settings under the entry object) or remove/update the subpath
exports in package.json; ensure emitted filenames/library.module format matches
the exports (update filename pattern and/or change export targets to .mjs) and
keep the experiments.outputModule and output.module settings intact so each
subpath produces its own dist/<subpath>/index(.mjs) file for proper resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant