jspm / generator

JSPM Import Map Generator
Apache License 2.0
160 stars 20 forks source link

jspm incorrectly considers a local dependency to be commonjs #259

Closed nex3 closed 1 year ago

nex3 commented 1 year ago
$ mkdir pkg
$ npm i @jspm/generator
$ mkdir subpkg
$ echo '{"name": "subpkg", "version": "0.0.0", "main": "index.js"}' > subpkg/package.json
$ echo 'console.log("hello!")' > subpkg/index.js
$ cat > generate.mjs
import { Generator } from '@jspm/generator';

const generator = new Generator({
  mapUrl: import.meta.url,
  defaultProvider: 'nodemodules',
  env: ['production', 'browser', 'module'],
});

await generator.install('./subpkg');
$ node generate.mjs
file:///tmp/pkg/node_modules/@jspm/generator/dist/generator-0681485c.js:2565
            throw new JspmError(`Unable to trace ${resolved}, as it is a CommonJS module. Either enable CommonJS tracing explicitly by setting "GeneratorOptions.commonJS" to true, or use a provider that performs ESM transpiling like jspm.io via defaultProvider: 'jspm.io'.`);
                  ^

JspmError: Unable to trace file:///tmp/pkg/subpkg/index.js, as it is a CommonJS module. Either enable CommonJS tracing explicitly by setting "GeneratorOptions.commonJS" to true, or use a provider that performs ESM transpiling like jspm.io via defaultProvider: 'jspm.io'.
    at TraceMap.visit (file:///tmp/pkg/node_modules/@jspm/generator/dist/generator-0681485c.js:2565:19)                                                      
    at async Generator.install (file:///tmp/pkg/node_modules/@jspm/generator/dist/generator-0681485c.js:3854:13)                                                      
    at async file:///tmp/pkg/generate.mjs:11:1 {
  jspmError: true,
  code: undefined
}

Node.js v19.7.0

It looks like jspm is considering my subpkg/index.js file to be a CommonJS file, despite not using any CommonJS features and being entirely possible to import via ESM. Even if I explicitly declare it as a non-CommonJS file using package.json conditional exports, jspm still throws this error. I can work around it by passing commonJS: true, but as a library author I'd like to avoid requiring my users to do that.

Note that while this is a toy example, my real use-case is more serious: I'm trying to distribute a library that will seamlessly support both CJS and ESM users with the same codebase. To do so, I use side effects when the library is loaded via ESM to pass out its exports without violating CJS syntax.

Bubblyworld commented 1 year ago

Thanks for the issue - I think this is a case of the generator being overzealous with CommonJS detection. At the moment the CJS warning is triggered if at any point the dependency tracer hits a CJS module. In this case since subpkg hasn't got "type": "module" set in its package.json, and because it's main file has the .js extension, the resolver treats it as a CJS module (see Node.js resolution rules, which jspm is an extension of).

But you're right, it only matters if the CJS module actually uses require/module/__dirname/etc. I think we can patch it quickly by checking if the module has any of those as unbound globals, and only throwing if it does.

nex3 commented 1 year ago

If the file is loaded through conditional exports for the type "import", it should probably also always be considered an ESM module.

guybedford commented 1 year ago

Fixed in https://github.com/jspm/generator/pull/261.