Skip to content

replace commmonjs2 to module for export #88

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
juah255 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

replace commmonjs2 to module for export #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
"name": "@wedevs/plugin-ui",
"version": "2.0.0",
"description": "Scoped, themeable UI components for WordPress plugins - ShadCN style",
"main": "dist/index.js",
"module": "dist/index.js",
"main": "dist/index.mjs",
"module": "dist/index.mjs",
"types": "dist/index.d.ts",
"sideEffects": [
"**/*.css"
],
"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.js",
"require": "./dist/index.js"
"import": "./dist/index.mjs",
"require": "./dist/index.mjs"
Comment on lines +14 to +15
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

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.

},
"./components/ui": {
"types": "./dist/components/ui/index.d.ts",
Expand Down
15 changes: 13 additions & 2 deletions webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,23 @@ module.exports = {
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',
Comment on lines 7 to +22
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

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.

environment: {
...( defaultConfig.output?.environment || {} ),
module: true,
dynamicImport: true,
},
clean: true,
},
Expand Down
Loading