Unused files published

[JounQin]

Bug description

.cjs files should be defined in exports, or remove them from dist

And @esbuild-kit/esm-loader is ESM-only, so we should remove .cjs files from dist

Reproduction

https://unpkg.com/browse/tsx@3.6.0/package.json

Environment

  System:
    OS: Linux 5.16 Ubuntu 20.04.4 LTS (Focal Fossa)
    CPU: (16) x64 AMD EPYC 7B13
    Memory: 33.70 GB / 62.80 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 16.15.1 - ~/.nvm/versions/node/v16.15.1/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.15.1/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.15.1/bin/npm
  npmPackages:
    tsx: ^3.6.0 => 3.6.0

Can you contribute a fix?

• I’m interested in opening a pull request for this issue.

[privatenumber]

I don't understand what the bug is.

[JounQin]

@privatenumber

Take dist/loader.cjs for example, it is not defined in exports field of package.json, it then is unavailable, require('tsx/dist/loader.cjs') throws.

[privatenumber]

They're there due to a (expected) limitation in Rollup that only allows specifying a format per build rather than file.

loader.cjs is not expected to work because Node.js loaders must be ESM files.

There might be room for improvement to remove unused files in terms of publish size, but I think it will be very an insignificant improvement.

AFAIK this isn't causing any bugs.

[JounQin]

Previously, I visit node_modules/tsx/dist folder and find loader.cjs and try to require('tsx/dist/loader.cjs') and it throws, so I think this is an unexpected behavior.

.cjs files should be defined in exports, or remove them from dist

And @esbuild-kit/esm-loader is ESM-only, so we should remove .cjs files from dist

[privatenumber]

That's expected behavior because in Node.js, export maps define what's possible to import. Not the distribution files.

[JounQin]

That's expected behavior because in Node.js, export maps define what's possible to import.

I understand this, but my point is these .cjs are useless and unexpected for users.

I can still hack to use absolute path to load .cjs which is not exported:

require(path.resolve(require.resolve('tsx'), '../loader.cjs'))

This will still throw because @esbuild-kit/esm-loader is ESM-only.

---

I agree

it will be very an insignificant improvement.

And we can have it, why not?

Also, if you agree, I can raise a PR for it.

[privatenumber]

Why are you trying to import loader.cjs?

When bundling will Rollup, it will create "chunk files" for code shared across entry points. The dist directory is bound to have files that are not importable.

This is a pkgroll concern so it should be fixed there. Any fix in tsx would be a duct-tape fix. I don't think this is easily fixable but sure.

[privatenumber]

Closing this because it would need to be fixed in https://github.com/privatenumber/pkgroll

Please re-open an issue there @JounQin