lukeed / polka

A micro web server so fast, it'll make you dance! :dancers:
MIT License
5.43k stars 174 forks source link

Firebase Cloud Functions possible fix. #152

Closed giuseppecrj closed 3 years ago

giuseppecrj commented 3 years ago

I was stuck trying to get polka running on firebase functions and came across a couple gotchas,

To get the lowest possible function working, this is the code:

const functions = require('firebase-functions');
const polka = require('polka')()

polka.get('/', (req, res) => {
    res.send('hello world')
})

exports.polka = functions.https.onRequest(polka.handler)

However I came across this error: TypeError: Cannot set property path of #<IncomingMessage> which has only a getter

After digging into the source code the issue seems to be here:

https://github.com/lukeed/polka/blob/master/packages/polka/index.js#L75

the simplest fix to get all of this to work was to change it to:

let path = req.path
let base = value(path = info.pathname);
// ....

Not sure if this is the intention but just wanted to provide feedback and see if we can get a version of this fix pushed. Cheers!

talentlessguy commented 3 years ago

You have to make those properties settable:

export const app = onRequest((request, response) => {
  makeReadonlySettable(request)
  polka.handler(request, response)
})

/** Hack for firebase functions request object, it was read only */
function makeReadonlySettable(req: Request) {
  return ['xhr', 'path'].forEach((key) => {
    Object.defineProperty(req, key, {
      get: function () {
        return this[`_${key}`]
      },
      set: function (val) {
        this[`_${key}`] = val
      }
    })
  })
}
lukeed commented 3 years ago

Yup – related to #97

Unfortunately all approaches listed are measurably slower. Future Polka version(s) will have per-environment builds that only pay these kids of prices when necessary.

giuseppecrj commented 3 years ago

Sounds great, thanks for the info! Closing issue.