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

build: remove non existing cjs dist from exports #69

Merged
merged 1 commit into from
Jul 17, 2023
Merged

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Apr 25, 2023

Currently listhen depends on ip-regex which is ESM-only. We could downgrade to a lower version of that package, but assuming we are using it, using listhen in a CJS context will throw this error:

[14:51:03]  ERROR  require() of ES Module /project/ip-regex@5.0.0/node_modules/ip-regex/index.js from /project/listhen@1.0.4/node_modules/listhen/dist/chunks/cert.cjs not supported.
Instead change the require of index.js in /project/listhen@1.0.4/node_modules/listhen/dist/chunks/cert.cjs to a dynamic import() which is available in all CommonJS modules.

  Instead change the require of index.js in node_modules/.pnpm/listhen@1.0.4/node_modules/listhen/dist/chunks/cert.cjs to a dynamic import() which is available in all CommonJS modules.
  at Object.<anonymous> (node_modules/.pnpm/listhen@1.0.4/node_modules/listhen/dist/chunks/cert.cjs:5:17)

@danielroe danielroe added the bug Something isn't working label Apr 25, 2023
@danielroe danielroe requested a review from pi0 April 25, 2023 13:54
@danielroe danielroe self-assigned this Apr 25, 2023
@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Merging #69 (4513fb9) into main (0b34c7a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #69   +/-   ##
=======================================
  Coverage   54.24%   54.24%           
=======================================
  Files           3        3           
  Lines         837      837           
  Branches       58       58           
=======================================
  Hits          454      454           
  Misses        383      383           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pi0 pi0 changed the title build: remove cjs output build: remove non existing cjs dist from exports Jul 17, 2023
@pi0 pi0 merged commit c2cd740 into main Jul 17, 2023
3 checks passed
@pi0 pi0 deleted the fix/remove-cjs branch July 17, 2023 14:33
@pi0
Copy link
Member

pi0 commented Jul 17, 2023

Build should be fixed by https://github.com/unjs/listhen to support CJS. It still makes sense to support to speed up CJS situations (like jiti to avoid transform..)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants