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

decouple start code from CLI #14

Closed
jdanyow opened this issue Dec 1, 2018 · 4 comments
Closed

decouple start code from CLI #14

jdanyow opened this issue Dec 1, 2018 · 4 comments

Comments

@jdanyow
Copy link
Collaborator

jdanyow commented Dec 1, 2018

Opening to continue discussion from #13 (comment)

What do you think about splitting start.js into two files:

  1. cli.js: argv, SIGHUP and file access related code
  2. index.js: exports a start and updateWorkerScript functions, re-exports the worker.js and server.js exports?
import { start, updateWorkerScript } from 'cloudflare-worker-local';

const options = {
  port: 4000,
  workerScript: '... javascript ...',
  upstreamHost: 'localhost:3000',
  kvStores: [],
  workerCount: 1
};

start(options);
...
...
updateWorkerScript('... js ...');

This would make the following scenario cleaner / less hacky:

import chokidar from 'chokidar';
import { access, F_OK } from 'fs';

const filename = 'dist/index.js';

let started = false;

async function handler() {
  const exists = await new Promise(resolve => access(filename, F_OK, e => resolve(!e)));
  if (exists && !started) {
    started = true;
    // TEMP: set args expected by cloudflare-worker-local
    process.argv[2] = filename;
    process.argv[3] = 'localhost:7001';
    process.argv[4] = '7000';
    import('cloudflare-worker-local/start.js');
  } else if (exists && started) {
    process.emit('SIGHUP');
  }
}

let timeout = 0;
function debouncedHandler() {
  clearTimeout(timeout);
  timeout = setTimeout(handler, 300);
}

chokidar.watch(filename)
  .on('add', debouncedHandler)
  .on('change', debouncedHandler);
@gja
Copy link
Owner

gja commented Dec 1, 2018

Hmmm, Lots of food for thought. I think we should be exporting the following things:

  • startServer(pathToWorker, {port}) (this automatically will updating the worker on SIGHUP)
  • Worker
  • startTestServer(workerContents, upstreamBehavior)

I've updated the readme, turns out watching and listening for SIGHUP is actually much easier than I thought.

@jdanyow
Copy link
Collaborator Author

jdanyow commented Dec 1, 2018

In my experience so far using cloudflare-worker-local with packagers like parcel and rollup, nodemon hasn't worked well. It leads to complex npm scripts to ensure the packager has built the worker script prior to starting cloudflare-worker-local. Similar to remy/nodemon#1297 and remy/nodemon#797.

The sighup approach seems to be ok for scenarios where the worker source code is a single javascript file.

For increased flexibility I think it would be great if the CLI related logic (process.argv, SIGHUP and fs.readFile(pathToWorker)) was decoupled from the core logic (start server / update worker).

@gja
Copy link
Owner

gja commented Dec 6, 2018

I think this will probably be the direction we go in. As a start, I've opened #19, which just exports (by default) Worker and createApp.

@gja
Copy link
Owner

gja commented Dec 6, 2018

If you want to use chokidar, the new pseudocode would look something like this

const {createApp} = require("cloudflare-worker-local")

const app = createApp(fs.readFileSync("/path/to/worker.js"), {})
app.listen(3000);

chokidhar.watch("/path/to/worker.js")
  .on('change', () => fs.readFile('/path/to/worker.js', buffer => app.updateWorker(bufferToString(buffer))))

@jdanyow jdanyow closed this as completed Dec 11, 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

No branches or pull requests

2 participants