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

Improve ESM support through CommonJS #50

Closed
bogeychan opened this issue Jun 2, 2023 · 4 comments
Closed

Improve ESM support through CommonJS #50

bogeychan opened this issue Jun 2, 2023 · 4 comments

Comments

@bogeychan
Copy link
Contributor

bogeychan commented Jun 2, 2023

At the moment it is not possible to import CommonJS into ESM because the exports defined in package.json (of all projects) are sorted incorrectly.


Example: (run with node main.js)

// main.js
import Elysia from 'elysia';

new Elysia.Elysia();

Error: (yes, i've "type": "module" in my package.json)

SyntaxError: Cannot use import statement outside a module

The sorting is important. Node.js & Deno apparently check from top to bottom. This means that if it is ESM-context, import is used instead of require or node.

Therefore node must move further up and point to CommonJS (in all exports):

{
  "bun": "./dist/index.js",
+ "node": "./dist/cjs/index.js"
  "require": "./dist/cjs/index.js",
  "import": "./dist/index.js",
  "default": "./dist/index.js",
- "node": "./dist/index.js"
}

Furthermore, type or runtime errors occur when CommonJS is imported into ESM, because SWC breaks named exports for ESM in CommonJS build (i.e. module.exports):

❌ SWC

(function (e, t) {
  for (var r in t) Object.defineProperty(e, r, { enumerable: !0, get: t[r] });
})(exports, {
  Elysia: function () {
    return h;
  },
  default: function () {
    return u;
  }
});

Runtime Error

import { Elysia } from 'elysia';
         ^^^^^^
SyntaxError: Named export 'Elysia' not found. The requested module 'elysia' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'elysia';
const { Elysia } = pkg;

OR Type error

import Elysia from 'elysia';

const app = new Elysia.Elysia() // Property 'Elysia' does not exist on type 'typeof Elysia'.ts(2339)

✅TSC (without SWC)

exports.Elysia = Elysia;
exports.default = Elysia;

I can make (or help you with) the changes to exports in all package.json, but I don't know how you wanna handle SWC.

An alternative would be to drop CommonJS and only support ESM → "moduleResolution": "nodenext" & "type": "module" (i.e. specify all imports and exports with .js extension). This would work for Node.js and Deno too.

@bogeychan
Copy link
Contributor Author

bogeychan commented Jun 2, 2023

For reference, I made basic adjustments in my fork

Just run bun run test:node

@SaltyAom
Copy link
Member

SaltyAom commented Jun 4, 2023

We will be using tsc to compile CommonJS to support module.exports syntax instead.

Should be released on 0.5.14 with this commit 69a130d

@bogeychan
Copy link
Contributor Author

That should be all.

  • Updating memoirist is not necessary as it is imported from CommonJS, so require is used anyway
  • I could not update @elysiajs/websocket because there are a bunch of type errors when upgrading Elysia.js to 0.5.12

@SaltyAom
Copy link
Member

SaltyAom commented Jun 7, 2023

For @elysiajs/websocket is migrated to the core package via elysia/ws, so no need to update the package.

That should be all for CommonJS, so I'm closing the issue.

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

No branches or pull requests

2 participants