Cloudflare worker with the same behavior as Node.js/Express middleware

[baoshan]

This PR creates a cloudflare folder which exports a Cloudflare worker that behaves exactly the same as the Node.js/Express middleware (because the same generalized HTTP handler is doing the job).

I think it’s simpler to keep middlewares for different platforms in a same repo to reduce cognition cost. Cloudflare has two types of worker: service-worker and module. The former requires the main field of package.json points to a worker while the module type supports specifying the path of the module in wrangler.toml. So this PR uses the module type.

Following environment variables are required/supported by the script:

  readonly CLIENT_TYPE?: ClientType; // defaults to "oauth-app"
  readonly CLIENT_ID: string;
  readonly CLIENT_SECRET: string;
  readonly PATH_PREFIX?: string; // defaults to "/api/github/oauth"

No automated tests are authored because I don’t know whether it’s worthy to import a simulator for Cloudflare workers such as miniflare. But I’d like to add automated tests if the maintainers believe it is necessary.

esbuild is used to bundle the script because I have no idea of how pika works (and it is no longer maintained). I’d like to change the bundler to pika or something else if necessary.

I have no idea of how to submit a PR against the previous one #247, so the diff is long. Only files in the cloudflare folders (plus wrangler.toml and package.json) are pertinent.

I’d like to catch up with a README section later. For quick test, I’ve deployed a live version to https://octokit-oauth-app.icmd.workers.dev.

Thanks.

[wolfy1339]

This isn't needed. Pika automatically includes the files in src/**/*

[baoshan]

Cloudflare worker accepts a single JS file. Please let me know if I need to use Pika to bundle the worker script. How should the pipeline be structured? Sorry, I’m new to Pika.

[wolfy1339]

Bundling should be left to end-users. We only compile the source down to plain JavaScript, which Pika handles.

[gr2m]

I have no idea of how to submit a PR against the previous one #247, so the diff is long. Only files in the cloudflare folders (plus wrangler.toml and package.json) are pertinent.

Sorry I missed your comment. I think you'll need to create a new pull request on your fork at baoshan/oauth-app.js@runtime-agnostic-middleware...baoshan:cloudflare-worker

[gr2m]

Could you add a README with usage instructions to the cloudeflare worker?

One thing I'm not sure about is whether we should read out environment variables within the code, I think that's an anti-pattern. I'd rather have normal options and let the user do the mapping to environment variables they defined.

[baoshan]

Thanks for the suggestions:

Here is the PR: baoshan#3

I expect users of oauth-app.js use below steps to instantiate a Cloudflare worker :

1. git clone && npm install
2. Fill in the account_id in wrangler.toml
3. wrangler secret put CLIENT_ID
4. wrangler secret put CLIENT_SECRET
5. wrangler publish

Do you suggest users need to write JavaScript code to bind clientID and clientSecret?

In the documentation: https://developers.cloudflare.com/workers/cli-wrangler/configuration#modules

Modules receive all bindings (KV Namespaces, Environment Variables, and Secrets) as arguments to the exported handlers. Previously, with the Service Worker format, these bindings were available as global variables.

To do that, users have to change src/middleware/cloudflare/index.ts unless another mechanism is injected.

I’ll add a README (maybe to /README.md since the instruction for createNodeMiddleware is there) once the code is reviewed.

Thanks again.

[gr2m]

You wouldn't deploy @octokit/oauth-app directly. Instead you would create a repository that uses @octokit/oauth-app.js and deploy that. We shouldn't add a wrangler.toml to the octokit/oauth-app.js repository.

For comparison, if you wanted to deploy an app to heroku or another deployment target that uses a long-lived node process, you could create a new repository with this code

const { OAuthApp, createNodeMiddleware } = require("@octokit/oauth-app");

const app = new OAuthApp({
  clientType: process.env.CLIENT_TYPE || "oauth-app",
  clientId: process.env.CLIENT_ID,
  clientSecret: process.env.CLIENT_SECRET,
});

require("http").createServer(createNodeMiddleware(app)).listen(process.env.PORT);

And then deploy that. That's where you set the environment variables, not in the source code of the library. Because using environment variables in the library code itself can cause unforeseen side effects.

For comparison, see the Probot adapter we created for Azure functions and the example app that uses that adapter

• https://github.com/probot/adapter-azure-functions
• https://github.com/probot/example-azure-function

Basically I'd suggest you create a repository such as https://github.com/gr2m/cloudflare-worker-github-app-example which uses the new cloudflare middleware in @octokit/oauth-app you are creating and make that the repository that is deployable to Cloudflare Workers, not @octokit/oauth-app itself.

Does that make sense?