Skip to content
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.

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

🐛 Bug: Properly dual-export CJS and ESM #7

Open
3 tasks done
JoshuaKGoldberg opened this issue Jan 19, 2025 · 4 comments · May be fixed by #13
Open
3 tasks done

🐛 Bug: Properly dual-export CJS and ESM #7

JoshuaKGoldberg opened this issue Jan 19, 2025 · 4 comments · May be fixed by #13
Labels
status: accepting prs Please, send a pull request to resolve this! 🙏 type: bug Something isn't working 🐛

Comments

@JoshuaKGoldberg
Copy link
Owner

Bug Report Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have pulled the latest main branch of the repository.
  • I have searched for related issues and found none that matched my issue.

Expected

I thought I'd set up dual-exporting CJS and ESM files properly in 6bf5c33, based on https://github.com/JoshuaKGoldberg/create-typescript-app/blob/c6714eeb02a5106eb5e4e4694f1e8b590444030d/docs/FAQs.md#how-can-i-add-dual-commonjs--ecmascript-modules-emit.

Actual

https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/actions/runs/12854886842/job/35839775194?pr=750 shows an error:

Run node ./lib/index.js
/home/runner/work/eslint-plugin-package-json/eslint-plugin-package-json/node_modules/.pnpm/[email protected]_@[email protected].[6](https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/actions/runs/12854886842/job/35839775194?pr=750#step:5:7)[email protected][email protected]_/node_modules/eslint-fix-utils/lib/index.cjs:18
__reExport(index_exports, require("./fixRemoveArrayElement.js"), module.exports);
                          ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /home/runner/work/eslint-plugin-package-json/eslint-plugin-package-json/node_modules/.pnpm/[email protected]_@[email protected][email protected][email protected]_/node_modules/eslint-fix-utils/lib/fixRemoveArrayElement.js from /home/runner/work/eslint-plugin-package-json/eslint-plugin-package-json/node_modules/.pnpm/[email protected]_@[email protected][email protected][email protected]_/node_modules/eslint-fix-utils/lib/index.cjs not supported.
Instead change the require of fixRemoveArrayElement.js in /home/runner/work/eslint-plugin-package-json/eslint-plugin-package-json/node_modules/.pnpm/[email protected]_@[email protected][email protected][email protected]_/node_modules/eslint-fix-utils/lib/index.cjs to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/home/runner/work/eslint-plugin-package-json/eslint-plugin-package-json/node_modules/.pnpm/[email protected]_@[email protected][email protected][email protected]_/node_modules/eslint-fix-utils/lib/index.cjs:18:2[7](https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/actions/runs/12854886842/job/35839775194?pr=750#step:5:8))
    at Object.<anonymous> (/home/runner/work/eslint-plugin-package-json/eslint-plugin-package-json/lib/rules/no-redundant-files.js:24:31)
    at Object.<anonymous> (/home/runner/work/eslint-plugin-package-json/eslint-plugin-package-json/lib/plugin.js:26:33)
    at Object.<anonymous> (/home/runner/work/eslint-plugin-package-json/eslint-plugin-package-json/lib/index.js:26:21) {
  code: 'ERR_REQUIRE_ESM'
}

Additional Info

Interestingly, that PR's test job passes. So it's not seemingly a blocker to using this package as an end-user. Just the CI of an implementing ESLint plugin.

💖

@michaelfaith
Copy link
Collaborator

I believe it's that main is pointing to the esm entry point. There should be a module that points to the esm entry point, and main points to the cjs entry point. I can take a look at it later, if you don't get to it sooner.

@michaelfaith
Copy link
Collaborator

michaelfaith commented Jan 19, 2025

oh! And I think this is also a result of the issue I mentioned in the PJV TS PR. Having .js explicitly in the import like that, is going to cause the .cjs files to try and require the esm versions of the files they import (which have the .js extension). I think there are a few ways we could deal with that. Like changing tsup to bundle, instead. If tsup supported rewriteRelativeImportExtensions (egoist/tsup#1271), it would make things a whole lot easier on us.

@JoshuaKGoldberg
Copy link
Owner Author

@all-contributors please add @JoshuaKGoldberg for bug.

🤖 Beep boop! This comment was added automatically by all-contributors-auto-action.
Not all contributions can be detected from Git & GitHub alone. Please comment any missing contribution types this bot missed.
...and of course, thank you for contributing! 💙

Copy link
Contributor

@JoshuaKGoldberg

I've put up a pull request to add @JoshuaKGoldberg! 🎉

JoshuaKGoldberg pushed a commit that referenced this issue Jan 19, 2025
Adds @JoshuaKGoldberg as a contributor for bug.

This was requested by JoshuaKGoldberg [in this
comment](#7 (comment))

---------

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
@michaelfaith michaelfaith linked a pull request Jan 19, 2025 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Please, send a pull request to resolve this! 🙏 type: bug Something isn't working 🐛
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants