jspm / project

Roadmap and management repo for the jspm project
161 stars 8 forks source link

CDN build error for `@codesandbox/sandpack-client` #251

Closed JayaKrishnaNamburu closed 1 year ago

JayaKrishnaNamburu commented 1 year ago

@codesnadbox/sandpack-client imports PREVIEW_LOADED_MESSAGE_TYPE from @codesandbox/nodebox And the package @codesandox/nodebox exports it too.

var src_exports = {};
--
622 | __export(src_exports, {
623 | INJECT_MESSAGE_TYPE: () => INJECT_MESSAGE_TYPE,
624 | MessageReceiver: () => MessageReceiver,
625 | MessageSender: () => MessageSender,
626 | Nodebox: () => Nodebox,
627 | PREVIEW_LOADED_MESSAGE_TYPE: () => PREVIEW_LOADED_MESSAGE_TYPE,
628 | createDebug: () => createDebug
629 | });
630 | module.exports = __toCommonJS(src_exports);

https://unpkg.com/browse/@codesandbox/nodebox@0.1.0/build/index.js

But, the jspm build of the package doesn't seem to export 🤔

https://ga.jspm.io/npm:@codesandbox/nodebox@0.1.0/build/index.js

Sandbox to test it https://jspm.org/sandbox#H4sIAAAAAAAAA41U21LbMBB95ysW9wXa2E5oaWlwMuGSYZohQ4tJW/qmyBsskCUjySFph3+vLDuBZCDtgy1rL2fPrnc32k4kNfMcITUZ725FiwNJ0t0CiDI0BGhKlEbT8Qoz8Q88pzDMcOyOhDuTKKzuSxdBMux4U4YPuVTGAyqFQWEhHlhi0k6CU0bRd5cGMMEMI9zXlHDstGyAKKwYRGOZzB3qtu/bA2AQfx3CGQpUxEgFX7ISH4Ykd9p+wgyMLs/bNh+T63YY3ixMg1udZwGT4ZvR3vf0+uz47nTYVPP49yAetVLav724at6ZKz07j+n9tHk0uTy+FoO9sRpc/MpyjEcXl0KeYXF99LO1/+3jaf/IRvR9R05TxXIDZSE7HnOUMpK7Qv1xvGqh9tq1wIp6VCaoiUjGchaWZ07onU85s3Wydt4yA7KkLvKsvcmttxc0g09hwrQJmUhwFmS32nMBHxsVEU1ljis8XojzTL1OVNhve/4vwdq81wxaQTMcF4wnNTNLrPEU45mPX9XKLwzjOuRsHFKFxKC/wDSY5dwK9AYSr+H1WsH74GAj6hozLhOi04BpvC8I3xBy1bD3Idi3Gb+UqyzMlChGNv7mJyNHuekoL9FqsMetxbt8orDqw7Lt6pGBfgxDmRQcIU5ZptuLgbHtqSGXfD5hnMPETlLmzDSMlXzQqDQ8MJNaFsCeeegid5cdYr1wRtF2/UmqZIZw8Pnd7to8ED0XFLSindeyRO1XcX1d0rOp7getqn/XVGXWQJXUWip2w0THI0KKeSYL7XXXM1+ZxwrFDSMsknlruUFcD8+Jmx2Y2DT+MZWHFT5AGMIPxQzaqjBRlw5KT0hRYQNsPW0AAhpzYrcPgi0zuopaoWcr4gExRrFxYXVSgElxgVJxD1wYuzW15BhwebOzynb3cOWHR2G1KO3edAv8L0acfrfYBQAA

guybedford commented 1 year ago

Named exports support uses the Node.js static analysis (https://github.com/nodejs/cjs-module-lexer). As you can see from that source, the exports aren't statically analyzable so wouldn't work in Node.js either.

JSPM supports a custom cjsNamedExports package.json field to override this:

{
  "cjsNamedExports": {
    "build/index.js": ["INJECT_MESSAGE_TYPE", "MessageReceiver", ...]
  }
}

This can also be added to the overrides repo if you don't want to amend the package itself - https://github.com/jspm/overrides/blob/main/overrides.json.

JayaKrishnaNamburu commented 1 year ago

Ah, got it. I forgot that jspm first looks for exports and then tries to generate one by parsing the package. Turns out they are exposing the cjs builds from the package.json. But they build esm as well. I will make them add exports pointing to esm modules.

Thanks guy 👍

JayaKrishnaNamburu commented 1 year ago

@guybedford but i have a little follow up on this. I added exports to the package. But the calculated exports still point to the main in package.json. Does the main always get's the priority over the exports ? https://ga.jspm.io/npm:@codesandbox/nodebox@0.1.1/package.json

guybedford commented 1 year ago

@JayaKrishnaNamburu it definitely seems to be pointing at the right thing in https://generator.jspm.io/#U2VgYGBkDM0rySzJSU1hcEjOT0ktTsxLScqv0M8DsoG0g4GeoZ4hAOGFJywpAA. "exports" always takes precedence over the "main".