Skip to content

fix(build): bundle pure esm package #133

New issue

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

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

Already on GitHub? # to your account

Merged
merged 2 commits into from
Mar 4, 2024
Merged

fix(build): bundle pure esm package #133

merged 2 commits into from
Mar 4, 2024

Conversation

Thiry1
Copy link
Contributor

@Thiry1 Thiry1 commented Mar 4, 2024

Summary

When using the openapi-typescript-code-generator as CJS, the following error occurs.

❯ node --input-type=commonjs -e "require('./dist/index.cjs')"
/Users/kei_sakamoto/workspace/openapi-typescript-code-generator/dist/index.cjs:1377
var DotProp2 = __toESM(require("dot-prop"), 1);
                       ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/kei_sakamoto/workspace/openapi-typescript-code-generator/node_modules/.pnpm/dot-prop@8.0.2/node_modules/dot-prop/index.js from /Users/kei_sakamoto/workspace/openapi-typescript-code-generator/dist/index.cjs not supported.
Instead change the require of index.js in /Users/kei_sakamoto/workspace/openapi-typescript-code-generator/dist/index.cjs to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/Users/kei_sakamoto/workspace/openapi-typescript-code-generator/dist/index.cjs:1377:24)
    at [eval]:1:1
    at Script.runInThisContext (node:vm:122:12)
    at Object.runInThisContext (node:vm:296:38)
    at [eval]-wrapper:6:24 {
  code: 'ERR_REQUIRE_ESM'
}

To resolve this issue, we need to bundle pure esm package.
However, tsup does not bundle packages that are listed in the dependencies.(see https://tsup.egoist.dev/#excluding-packages , egoist/tsup#628 (comment))
As a workaround, I moved the pure ESM package to devDependencies.

Test Plan

verify that the package can be loaded in both ESM and CJS with the following command.

node --input-type=module -e "import {CodeGenerator} from './dist/index.js'"
node --input-type=commonjs -e "require('./dist/index.cjs')"

@Thiry1
Copy link
Contributor Author

Thiry1 commented Mar 4, 2024

Some tests are failing.I am investigating it.

It only occurs locally.

@Himenon
Copy link
Owner

Himenon commented Mar 4, 2024

Thank you for the pull request. Is there anything we should include in our README about this change?

@Thiry1

@Himenon Himenon self-requested a review March 4, 2024 06:03
@Thiry1
Copy link
Contributor Author

Thiry1 commented Mar 4, 2024

@Himenon

Is there anything we should include in our README about this change?

I believe there is no impact on users, so I think it is unnecessary to add anything to the README.

@Himenon
Copy link
Owner

Himenon commented Mar 4, 2024

Yes, sir. Thank you!

@Himenon Himenon merged commit a3f034d into Himenon:main Mar 4, 2024
@Thiry1 Thiry1 deleted the fix/cjs branch March 4, 2024 06:11
@Himenon
Copy link
Owner

Himenon commented Mar 4, 2024

This changes was included in v1.0.9

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants