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

Fix CJS package masquerading as ESM on import #358

Closed
wants to merge 2 commits into from

Conversation

Philzen
Copy link
Contributor

@Philzen Philzen commented Dec 11, 2024

May fix #357

How to generate the required .d.cts using tsup or tsc went over the top of my head, however npx publint advised that it would suffice to duplicate the .d.ts file for that (see commit message of b5d562a for a detailed explanation).

Copy link

changeset-bot bot commented Dec 11, 2024

⚠️ No Changeset found

Latest commit: d50ed0e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@CLAassistant
Copy link

CLAassistant commented Dec 11, 2024

CLA assistant check
All committers have signed the CLA.

This is supposed to work according to `npx publint`:
> 1. pkg.exports["."].require types is not exported. Consider adding
> pkg.exports["."].require.types to be compatible with TypeScript's
> "moduleResolution": "bundler" compiler option. Note that you cannot
> use "./dist/index.d.ts" because it has a mismatching format. Instead,
> you can duplicate the file and use the .cts extension,
> e.g. pkg.exports["."].require.types: "./dist/index.d.cts"
@Philzen Philzen force-pushed the fix-publint-esm-cjs-mask branch from b5d562a to d50ed0e Compare December 11, 2024 06:37
@Philzen
Copy link
Contributor Author

Philzen commented Dec 11, 2024

I've added livekit-server-sdk into the project that exposed #357 as a local path using this branch … and can confirm that the warning is gone 🙏

@Philzen Philzen changed the title Fix publint esm cjs mask Fix CJS package masquerading as ESM on import Dec 11, 2024
@lukasIO
Copy link
Contributor

lukasIO commented Dec 11, 2024

thanks for this, I didn't see it and opened #359 which uses tsup directly to generate these, so we don't have to manually copy the files around. Thanks for raising the issue and offering a fix!

@Philzen
Copy link
Contributor Author

Philzen commented Dec 11, 2024

opened #359 which uses tsup directly to generate these, so we don't have to manually copy the files around.

That's of course the way (and feels much more trustworthy meaning less hacky). So at least i learned what i was looking for was hidden behind the dts: true option all along 😉

@Philzen Philzen closed this Dec 11, 2024
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.

Using ESM-style imports from livekit-server-sdk gives warnings ("current file is a CommonJS module")
3 participants