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

"[ERROR] require() of ES Module node_modules/.pnpm/nanoid@5.0.4/node_modules/nanoid/index.js from node_modules/.pnpm/elysia@0.8.10_@sinclair+typebox@0.32.12_typescript@5.3.3/node_modules/elysia/dist/cjs/ws/index.js not supported. Instead change the require of node_modules/.pnpm/nanoid@5.0.4/node_modules/nanoid/index.js in node_modules/.pnpm/elysia@0.8.10_@sinclair+typebox@0.32.12_typescript@5.3.3/node_modules/elysia/dist/cjs/ws/index.js to a dynamic import() which is available in all CommonJS modules." when integrating with Astro #441

Closed
dtinth opened this issue Jan 24, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@dtinth
Copy link

dtinth commented Jan 24, 2024

What version of Elysia.JS is running?

0.8.10

What platform is your computer?

Darwin 23.2.0 arm64 arm

What steps can reproduce the bug?

I follow the docs here: https://elysiajs.com/integrations/astro.html

Steps as follows:

  1. Create a new Astro app with create-astro
  2. Install Elysia: pnpm add elysia @sinclair/typebox
  3. Integrate Elysia with an Astro app:
// src/pages/services/[...rest].ts
import type { APIRoute } from "astro";

import { Elysia } from "elysia";

const app = new Elysia().get("/services/hello", () => ({ ok: true }));

export const ALL: APIRoute = ({ request }) => {
  return app.fetch(request);
};

Note

The docs used export const GET = handle; export const POST = handle but now Astro supports the ALL method. Maybe the docs can be updated to use more idiomatic pattern?

  1. Run pnpm dev to start astro dev server.

What is the expected behavior?

The app should work. Navigating to http://localhost:4321/services/hello should work.

image

What do you see instead?

When trying to access the endpoint, this error is generated instead:

Error [ERR_REQUIRE_ESM]: require() of ES Module node_modules/.pnpm/nanoid@5.0.4/node_modules/nanoid/index.js from node_modules/.pnpm/elysia@0.8.10_@sinclair+typebox@0.32.12_typescript@5.3.3/node_modules/elysia/dist/cjs/ws/index.js not supported.
Instead change the require of node_modules/.pnpm/nanoid@5.0.4/node_modules/nanoid/index.js in node_modules/.pnpm/elysia@0.8.10_@sinclair+typebox@0.32.12_typescript@5.3.3/node_modules/elysia/dist/cjs/ws/index.js to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (node_modules/.pnpm/elysia@0.8.10_@sinclair+typebox@0.32.12_typescript@5.3.3/node_modules/elysia/dist/cjs/ws/index.js:5:18)

Additional information

It seems that Elysia is compiled into CommonJS and is always preferred in Node.js

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

Note the order of conditional exports. node comes before import. This means that the CJS version of Elysia will be imported in Node.js regardless of whether Elysia.js is required or imported.

However, the CJS version of Elysia.js is currently broken, as the ws module tries to require the nanoid package, which is ESM-only.

Note these 2 points in the docs:

  • "node" - matches for any Node.js environment. Can be a CommonJS or ES module file. In most cases explicitly calling out the Node.js platform is not necessary.
  • Within the "exports" object, key order is significant. During condition matching, earlier entries have higher priority and take precedence over later entries.

I currently work around this issue by patching node_modules/elysia/dist/cjs/index.js to comment out the line that imports ws.

image

This made the app work for now, but not ideal to have to patch a dependency like this.

Maybe Elysia should have an integration test suite that ensures it works with Astro? (As well as other environments like Cloudflare Workers? (It was broken in earlier version.))

@dtinth dtinth added the bug Something isn't working label Jan 24, 2024
@SaltyAom
Copy link
Member

SaltyAom commented Jan 26, 2024

This should have been fixed with f7e067f, published on 0.8.13

Let me know if this fixes your problem.

@SaltyAom SaltyAom self-assigned this Jan 26, 2024
@dtinth
Copy link
Author

dtinth commented Jan 26, 2024

@SaltyAom Right now the package can be imported in Node.js, but they wouldn't compile in Astro.

Problem 1: Astro compilation

When running astro dev, I get this error:

18:22:31 [ERROR] Cannot find module 'elysia' imported from 'src/pages/services/[...segments].ts'
  Stack trace:
    at nodeImport (file://node_modules/.pnpm/vite@5.0.12_@types+node@20.11.6/node_modules/vite/dist/node/chunks/dep-9A4-l-43.js:50892:25)
    at eval (src/pages/services/[...segments].ts:3:50)

This seems to happen because Vite does not look at exports when resolving modules. Solution is to add "main" to package.json:

image

Problem 2: TypeScript

In VS Code, TypeScript complains:

Could not find a declaration file for module 'elysia'. 'node_modules/.pnpm/elysia@0.8.13_typescript@5.3.3/node_modules/elysia/dist/index.mjs' implicitly has an 'any' type.

This is because when "exports" property is present, Node.js will ignore "main" field, and TypeScript, following Node’s behavior, will ignore the "types" field. Docs.

If both "exports" and "main" are defined, the "exports" field takes precedence over "main" in supported versions of Node.js.

Solution is to add "types" to "exports".* object in package.json

image

@SaltyAom
Copy link
Member

My bad for accidentally deleting the types field, also til Vite only bundles with main field.

Should have been fixed with 7427079 published on 0.8.14.

I have verified that there's no problem with types and bundling on my end.

Screenshot 2567-01-26 at 20 10 56

@dtinth
Copy link
Author

dtinth commented Jan 26, 2024

It works now, thank you for a speedy fix! 🙏

@dtinth dtinth closed this as completed Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants