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

Allow alternative server by options #33

Closed
wants to merge 3 commits into from
Closed

Allow alternative server by options #33

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 3, 2018

Custom http server can be passed to options.
It has check for server in options to have .createServer method or else use default

Update examples where possible for usage of server option.
Excluded https as it takes options for ssl.

Cleanup of #13

Example

import http from 'somemagic-http-server';

polka({ server: http });

@roblav96
Copy link

@lukeed This is a very much needed feature. +1 merge

@lukeed
Copy link
Owner

lukeed commented Apr 22, 2018

Sorry for the delay on this 🙈 I've had something similar locally for a little while (thanks to @ThiccCat for the initial idea).

This PR doesn't account for the different createServer signatures from https and http2 modules. They need (opts, handler) -- and I imagine some other custom servers may do the same.

The only things we actually care about wrt the server are (1) passing it the Polka handler and (2) calling listen on it.

Because of this, my change is to pass in an actual server to Polka options if we're supposed to hook into it. Otherwise, if nothing is passed, a default http server is created when listen is called.

Attaching to https may look like this:

const https = require('https');
const polka = require('polka');

const server = https.createServer({ key, cert });
const app = polka({ server }).get('...', noop);

await app.listen(PORT); // this is where Polka attaches "handler" to "server"

Similarly, for uws custom http:

const { http } = require('uws');
const polka = require('polka');

// OPTION 1 :: No changes
const { handler } = polka().get('/', noop);
http.createServer(handler).listen(PORT, err => {
  console.log(`> Running on localhost:${PORT}`);
});

// OPTION 2 :: Pass server into Polka
const server = http.createServer();
polka({ server }).get('/', noop).listen(PORT).then(() => {
  console.log(`> Running on localhost:${PORT}`);
});

Lastly, for basic use, including Polka sub-applications, there's no change at all:

// no http server here
const sub = polka().get('/', noop); 

// no http server here either
const main = polka().use('/users', sub).get('/', noop); 

await main.listen(PORT);
//=> NOW only "main" app has a server booted

@lukeed lukeed added this to the 0.4.0 milestone Apr 22, 2018
@lukeed lukeed closed this in 276056c Apr 22, 2018
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

Successfully merging this pull request may close these issues.

2 participants