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

Cors? #8

Open
JuanCaicedo opened this issue Jan 20, 2018 · 3 comments
Open

Cors? #8

JuanCaicedo opened this issue Jan 20, 2018 · 3 comments

Comments

@JuanCaicedo
Copy link

Hi there! I'm trying to use fs-router but am having problems with needing to set up an OPTIONS route. Some more details.

I've used fs-router to set up a POST route at /sync. I've used micro-cors to set up cors. However when I send a request from my client, I see a 404 at OPTIONS /sync. This is because the browser sends an OPTIONS preflight request to the server first.

I think the solution would be to set up an OPTIONS handler as well, but I'm not sure what to do there. Have you ever encountered this problem, or do you have any other suggestions for doing cors with fs-router?

@JuanCaicedo
Copy link
Author

JuanCaicedo commented Jan 20, 2018

Never mind, I was able to get it working along these lines

const { send } = require('micro')
const cors = require('micro-cors')()

const POST = async (req, res) => {
  send(res, 200, { status: 'OK' })
}

module.exports = {
  POST: cors(POST),
}

@JuanCaicedo
Copy link
Author

JuanCaicedo commented Jan 20, 2018

Sorry, I think I had some problems with how the browser cached the results of those requests. I think this is the code that works

const { send } = require('micro')
const cors = require('micro-cors')()

const POST = async (req, res) => {
  send(res, 200, { status: 'OK' })
}

module.exports = {
  POST: cors(POST),
  OPTIONS: cors((req, res) => send(res, 200)),
}

This puts me back to thinking, should we manually have to match the options route? I think both express cors and micro-cors don't have to do that.

This might be a feature request you don't want to deal with. If you feel okay with users just adding the above OPTIONS, feel free to close this issue 😀

@JuanCaicedo JuanCaicedo reopened this Jan 20, 2018
@jesseditson
Copy link
Owner

This is interesting! When I've added cors to fs-router projects in the past, I'll usually install the cors handler on the base route, which will handle adding the OPTIONS requests - something like this:

// index.js
const { send } = require('micro')
const cors = require('micro-cors')()
let match = require('fs-router')(__dirname + '/routes')

module.exports = cors(async function(req, res) {
  let matched = match(req)
  if (matched) {
    try {
      return await matched(req, res)
    } catch (e) {
      console.error(e.stack)
      return send(res, 500, { error: `Server error: ${e.message}` })
    }
  }
  send(res, 404, { error: 'Not found' })
})

But I could see it being useful to only install cors handlers on a specific route - what would you imagine that API looking like? Off the top of my head, it seems like it may be a bit overkill to reflect the route to figure out if it wants an OPTIONS handler, but I'm probably just not thinking of the simpler solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants