`set.headers` ignored when Elysia is bundled

Coachonko commented 2 months ago

What version of Elysia.JS is running?

1.0.15

What platform is your computer?

Linux 5.14.0-362.24.1.el9_3.x86_64 x86_64 x86_64

What steps can reproduce the bug?

import Elysia from 'elysia'

function myHandler (ctx) {
  try {
    const body = `<!DOCTYPE html><head></head><body>HTML</body>`
    ctx.set.headers['Content-Type'] = 'text/html; charset=utf-8'
    return body
  } catch (err) {
    console.error(err)
  }
}

export function tryFiles (ctx, publicDir, handler) {
  if (ctx.path === '/') {
    return handler(ctx)
  }

  const absolutePath = join(process.cwd(), publicDir, ctx.path)

  const file = Bun.file(absolutePath)
  if (file.size === 0) {
    return handler(ctx)
  }

  return file
}

new Elysia()
  .get('*', ctx => {
    if (process.env.BUN_ENV === 'production') {
      return myHandler(ctx)
    }
    return tryFiles(ctx, 'build/browser', myHandler)
  })
  .on('start', () => console.log(`Running on port ${process.env.PORT}`))
  .listen(process.env.PORT)

This code works as intended when run by Bun without bundling. If this code is bundled bun build ./index.js --outdir ./build --target=bun --public-path /_statics/ then the browser receives Content-Type text/plain instead of text/html

{
  "name": "elysia-bugs",
  "module": "index.js",
  "type": "module",
  "devDependencies": {
    "@types/bun": "latest"
  },
  "scripts": {
    "build": "bun build ./index.js --outdir ./build --target=bun --public-path /_statics/",
    "dev": "bun run build && PORT=8080 BUN_ENV=production bun run build/index.js",
    "start": "bun run build && PORT=8080 bun run build/index.js"
  },
  "peerDependencies": {
    "typescript": "^5.0.0"
  },
  "dependencies": {
    "elysia": "^1.0.15"
  }
}

I need the bundle to work, please help me

What is the expected behavior?

built and not-built app work the same

What do you see instead?

built app is broken

Additional information

This may be a problem with bun: In my debugging journey, I have combined the functions 2 handlers above (tryFiles and myHandler) into one and this issue disappears. However, when using the --minify flag, the same issue appears again.

bogeychan commented 2 months ago

@Coachonko, as a workaround, you can set new Elysia({ aot: false }).

@SaltyAom, inferBodyReference needs a whole rewrite to be more robust.

this line only checks for myHandler(ctx)

https://github.com/elysiajs/elysia/blob/968650becd19850e4f33dca273d9e5838c937d70/src/sucrose.ts#L415-L416

and not these cases:

myHandler("", ctx)
myHandler(ctx, "")
myHandler("", ctx, "")
myHandler( ctx )
myHandler( ctx , "" )
myHandler( "" , ctx )
myHandler( "" , ctx , "" )

Bun build turns the code in something like this:

var myHandler = function (ctx) {
    try {
        const body = `<!DOCTYPE html><head></head><body>HTML</body>`
        ctx.set.headers['Content-Type'] = 'text/html; charset=utf-8'
        return body
    } catch (err) {
        console.error(err)
    }
}
function tryFiles(ctx, publicDir, handler5) {
    if (ctx.path === '/') {
        return handler5(ctx)
    }
    const absolutePath = join(process.cwd(), publicDir, ctx.path)
    const file = Bun.file(absolutePath)
    if (file.size === 0) {
        return handler5(ctx)
    }
    return file
}
new Elysia()
    .get('*', (ctx) => {
        if (false) {
        }
        return tryFiles(ctx, 'build/browser', myHandler)
    })
    .on('start', () => console.log(`Running on port 8080`))
    .listen(8080)

thejasonxie commented 2 months ago

@Coachonko, as a workaround, you can set new Elysia({ aot: false }).

@SaltyAom, inferBodyReference needs a whole rewrite to be more robust.

this line only checks for myHandler(ctx)

https://github.com/elysiajs/elysia/blob/968650becd19850e4f33dca273d9e5838c937d70/src/sucrose.ts#L415-L416

and not these cases:

* `myHandler("", ctx)`

* `myHandler(ctx, "")`

* `myHandler("", ctx, "")`

* `myHandler(     ctx     )`

* `myHandler(     ctx     ,     ""     )`

* `myHandler(     ""     ,     ctx     )`

* `myHandler(     ""     ,     ctx     ,     ""     )`

Bun build turns the code in something like this:

var myHandler = function (ctx) {
  try {
      const body = `<!DOCTYPE html><head></head><body>HTML</body>`
      ctx.set.headers['Content-Type'] = 'text/html; charset=utf-8'
      return body
  } catch (err) {
      console.error(err)
  }
}
function tryFiles(ctx, publicDir, handler5) {
  if (ctx.path === '/') {
      return handler5(ctx)
  }
  const absolutePath = join(process.cwd(), publicDir, ctx.path)
  const file = Bun.file(absolutePath)
  if (file.size === 0) {
      return handler5(ctx)
  }
  return file
}
new Elysia()
  .get('*', (ctx) => {
      if (false) {
      }
      return tryFiles(ctx, 'build/browser', myHandler)
  })
  .on('start', () => console.log(`Running on port 8080`))
  .listen(8080)

The workaround works but leads into other issues like mapResponse not working properly when gzipping. An issue was opened about this https://github.com/elysiajs/elysia/issues/638

Hope this can be fixed soon! Thanks!

thejasonxie commented 2 months ago

Another issue for when aot:false, is that when using the error function in the onBeforeHandle for authentication, we get "undefined is not a function (near '...error32...')"

desplmfao commented 2 months ago

im not sure if this is a part of being bundled but i am having issues with setting headers in onError (it doesnt set any headers, ctx.set.headers logs the proper headers)

disabling aot fixes this too ^

SaltyAom commented 3 weeks ago

Should be fixed on ea5d133, to be released in 1.1

Please leave this issue until 1.1 stable is released

elysiajs / elysia