Packages incorrectly reported as not-ESM when they are

isker commented 1 month ago

Problem

On https://pkg-size.dev/cm6-graphql@v0.0.15, https://www.npmjs.com/package/cm6-graphql/v/0.0.15?activeTab=code and https://www.npmjs.com/package/graphql-language-service/v/5.2.1?activeTab=code are both reported as not exporting ESM when they do, as you can see from browsing the code on those npm links.

Expected behavior

ESM is accurately detected.

Minimal reproduction URL

These questions are invalid on this project and should be removed

Version

These questions are invalid on this project and should be removed

Node.js version

These questions are invalid on this project and should be removed

Package manager

N/A

Operating system

macOS

Bugs are expected to be fixed by those affected by it

[ ] I'm interested in working on this issue

Compensating engineering work financially will speed up resolution

[ ] I'm willing to offer financial support

privatenumber commented 1 month ago

ESM is detected by whether Node will run the code in Module mode, not by syntax.

Neither of those packages correctly follow Node's criteria for running their ESM code in Module mode.

isker commented 1 month ago

I see, so pkg-size.dev is not measuring the behavior of bundling packages for web browsers.

privatenumber commented 1 month ago

ESM/CJS has nothing to do with bundling for browsers. Most bundlers handle both.

Pkg Size can be used to measure bundle size for browsers--depends on your criteria for what that means.

Majority of the time, this has more to do with dynamically adjusting the package code with polyfills to make a Node package work in the browser.

isker commented 1 month ago

Based on what you've said, pkg-size.dev is detecting whether a package includes ESM solely by looking for "type": "module", which is unrelated to bundling. I had thought this was a bug, because pkg-size.dev says "No ES modules", even though cm6-graphql declares an ES module entrypoint. ES modules are present in this package's distribution on NPM.

ESM/CJS has nothing to do with bundling for browsers. Most bundlers handle both.

ESM vs. CJS is important when it comes to bundling for browsers. It's challenging for bundlers to statically analyze CJS modules in order to do treeshaking, compared to ESM. For bundlers, choosing ESM entrypoints (in packages that have them) over CJS entrypoints is thus important to achieve optimal bundle size. As a developer targeting browsers, choosing NPM dependencies that have ESM entrypoints in their package.jsons over those that do not can be advantageous for the same reasons.

I had thought pkg-size.dev was telling me that cm6-graphql was CJS-only, which is an indication that, when targeting for web browsers, it should be avoided if possible. But I know that, for example, rollup would select the ESM entrypoint from cm6-graphql's package.json over the CJS one, even when rollup is configured with CJS support.

privatenumber commented 1 month ago

There's a lot to cover and I don't have time to teach the details so I'll get to the point:

Those packages don't export ESM by this spec.
Again, "bundling for browsers" is subjective. You seem to specifically mean "tree-shaken" which I never would've guessed. Tree-shaking isn't limited to browsers because when I bundle package for Node.js, I expect them to be tree-shaken too.
Whether CJS can be tee-shaken depends on the bundler. Webpack does it perfectly well as long as exports are written with the same constraints as ESM exports.
The benefits of tree-shaking depends on the package and how much they rely on tree shaking. This is not a rule you can apply every time: "ESM vs. CJS is important when it comes to bundling for browsers." This is also why I made pkg-size—so you can actually see the exact impact of tree-shaking on a given package instead of assuming.
Whether a bundler loads CJS or ESM depends on its resolution logic. Just like tree-shaking, it depends on whether your bundler has implemented it. Today, the convention is to follow export conditions, which pkg-size supports.
Pkg Size uses esbuild to bundle, which supports export conditions and IIRC, only supports tree-shaking in ESM. In the "Bundle size" section, you can actually click on the named exports to toggle whether to include them in the bundle. In the case with cm6-graphql@v0.0.15, it seems like tree-shaking barely offers any benefit.

So, by your definition of "bundling for the web" to mean "tree-shaken ESM": yes, pkg-size can calculate the bundle size accurately.

(Locking the thread as I'm not trying to offer free ESM/bundling behavior lessons, but hope this helped)

privatenumber / pkg-size.dev