koajs / koa-body

koa body parser middleware
MIT License
947 stars 131 forks source link

[fix] @6.0.0 koaBody is not a function #215

Closed risu-p closed 1 year ago

risu-p commented 1 year ago

Describe the bug

Node.js version: v16.14.2 Description: @6.0.0 koaBody is not a function

Actual behavior

after install koa-body@6.0.0, get error: koaBody is not a function back to koa-body@5.0.0 everything is fine

Expected behavior

need a doc for @6.0.0 😊

Code to reproduce

const Router = require("koa-router");
const koaBody = require('koa-body');

const router = new Router();

router.post(
    "xxx", 
    koaBody({
        multipart: true,
        formidable: {
            multiples: false,
            maxFieldsSize: 10*1024*1024,
        },
    }), 
    async (ctx) => {
        ...
    }
);
sschen86 commented 1 year ago

const koaBody = require('koa-body').default; use this is fine

TheDadi commented 1 year ago

@risu-p As @sschen86 mentioned correctly koa-body@6.0.0 requires you to add the .default in your require statement. @MarkHerhold There are multiple ways of solving this issue

  1. Get rid of the default export and make it a named export e.g.
    • const { createKoaBodyMiddleware } = require('koa-body');
    • import { createKoaBodyMiddleware } from 'koa-body'; which is compatible with both commonJS and module syntax.
  2. Leave it as it is now and update the README.md to match the new require statement. (an example of an other module that did this -> https://github.com/conventional-changelog/commitlint/pull/2425)
  3. Leave the default export as is and add an additional named export for syntactic sugaring, which then allows to either
    • const { createKoaBodyMiddleware } = require('koa-body');
    • import { createKoaBodyMiddleware } from 'koa-body'; or
    • const createKoaBodyMiddleware = require('koa-body').default;
    • import createKoaBodyMiddleware from 'koa-body';

You decide 🦺

This is only a problem when using the module with commonJS require not for import / export

lbesson commented 1 year ago

It is currently also not possible to use koaBody in a typescript esm project.

If you import koaBody from 'koa-body'; then at runtime you'll get a TypeError: koaBody is not a function but you cannot use koaBody.default since default is not typed

TheDadi commented 1 year ago

@lbesson but this was also not possible before since it never exported an esm module or am i wrong?

lbesson commented 1 year ago

@TheDadi I'm still looking at it. But I can confirm that this was working with versions 5.0.0 (and previous ones) with my setup (typescript + esm), and not anymore since 6.0.0 (see for example https://github.com/c2corg/c2c_images/pull/83). It can normally import both commonjs or esm modules without issues.

I'll investigate whenever I have time, and report whatever I find. In any case, I think accessing createKoaBodyMiddleware with a named import would solve my issue.

MarkHerhold commented 1 year ago

I think we can just add a named export and change the docs to reflect that as the preferred usage pattern. I'm on vacation at the moment but can review a PR

MarkHerhold commented 1 year ago

published + koa-body@6.0.1

damianobarbati commented 1 year ago

@MarkHerhold is this the way now (ESM)?

import body from 'koa-body';

const app = new koa();
app.use(body.default());

Because is a little bit sucky 😢

MarkHerhold commented 1 year ago

@damianobarbati try

import { koaBody } from 'koa-body';

saschanaz commented 1 year ago

Something is weird here, if you try await import("koa-body") in the console:

> await import("koa-body")
[Module: null prototype] {
  HttpMethodEnum: {
    POST: 'POST',
    GET: 'GET',
    PUT: 'PUT',
    PATCH: 'PATCH',
    DELETE: 'DELETE',
    HEAD: 'HEAD'
  },
  KoaBodyMiddlewareOptionsSchema: ZodObject {
    spa: [Function: bound safeParseAsync] AsyncFunction,
    superRefine: [Function: bound _refinement],
    _def: {
      shape: [Function: shape],
      unknownKeys: 'strip',
      catchall: [ZodNever],
      typeName: 'ZodObject'
    },
    parse: [Function: bound parse],
    safeParse: [Function: bound safeParse],
    parseAsync: [Function: bound parseAsync] AsyncFunction,
    safeParseAsync: [Function: bound safeParseAsync] AsyncFunction,
    refine: [Function: bound refine],
    refinement: [Function: bound refinement],
    optional: [Function: bound optional],
    nullable: [Function: bound nullable],
    nullish: [Function: bound nullish],
    array: [Function: bound array],
    promise: [Function: bound promise],
    or: [Function: bound or],
    and: [Function: bound and],
    transform: [Function: bound transform],
    default: [Function: bound default],
    describe: [Function: bound describe],
    isNullable: [Function: bound isNullable],
    isOptional: [Function: bound isOptional],
    _cached: null,
    nonstrict: [Function: passthrough],
    augment: [Function (anonymous)],
    extend: [Function (anonymous)]
  },
  __esModule: true,
  default: {
    koaBody: [Function: koaBody],
    HttpMethodEnum: [Getter],
    KoaBodyMiddlewareOptionsSchema: [Getter],
    default: [Function: koaBody]
  },
  koaBody: [Function: koaBody]
}

default should be koaBody but somehow the default is the module object again.

PatrickBauer commented 1 year ago

@saschanaz I was really confused by this too when I tried to understand what is happening. As someone who wants to understand the things they use, I tried to find out what happens.

The NodeJS ESM loader supports importing CommonJS Modules (like koa-body). While importing it takes all named exports and puts them into a default export. This happens here: https://github.com/nodejs/node/blob/ecde9d9640e9b590a153c78b7dacc2739337f2a8/lib/internal/modules/esm/translators.js#L256

Its kind of a compatibility layer. What koa-body is doing right now (exporting "default" inside a CJS file) is not supported like they expected and will produce the error that was encountered here.

More information on why this is happening: https://github.com/nodejs/node/issues/48899 https://github.com/nodejs/node/pull/33416 https://github.com/nodejs/node/pull/35249

Bundlers take care of this as far as I understand, but the NodeJS default behavior for importing CJS into EMS is different.