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

set.headers ignored when Elysia is bundled #625

Closed
einar-hjortdal opened this issue May 1, 2024 · 5 comments
Closed

set.headers ignored when Elysia is bundled #625

einar-hjortdal opened this issue May 1, 2024 · 5 comments
Labels
bug Something isn't working pending Should be resolved in next release

Comments

@einar-hjortdal
Copy link

einar-hjortdal commented May 1, 2024

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
Copy link
Contributor

bogeychan commented May 13, 2024

@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)

elysia/src/sucrose.ts

Lines 415 to 416 in 968650b

// ! Function is passed to another function, assume as all is accessed
if (code.includes('(' + alias + ')')) {

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
Copy link
Contributor

thejasonxie commented May 16, 2024

@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)

elysia/src/sucrose.ts

Lines 415 to 416 in 968650b

// ! Function is passed to another function, assume as all is accessed
if (code.includes('(' + alias + ')')) {

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 #638

Hope this can be fixed soon! Thanks!

@thejasonxie
Copy link
Contributor

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
Copy link

desplmfao commented May 22, 2024

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
Copy link
Member

SaltyAom commented Jul 2, 2024

Should be fixed on ea5d133, to be released in 1.1

Please leave this issue until 1.1 stable is released

@SaltyAom SaltyAom added the pending Should be resolved in next release label Jul 2, 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 pending Should be resolved in next release
Projects
None yet
Development

No branches or pull requests

5 participants