sqrl-lang / sqrl

A safe, stateful rules language for event streams
Apache License 2.0
113 stars 10 forks source link

Deno support for sqrl core library? #51

Open thoughtpolice opened 1 year ago

thoughtpolice commented 1 year ago

Hey, thanks for posting this on HN, it looks really cool! I was wondering was if you'd accept some small tweaks to get the core sqrl library working with Deno? I've been exploring it for deploying JavaScript libraries and I think SQRL would be a great fit for this; it makes it super easy to deploy things like custom servers based on JavaScript APIs like this.

Deno has support for importing some subset of npm packages with the npm: qualifier on an import. So, we might expect the following typescript program to work:

import { Instance, Execution, AT } from "npm:sqrl@0.6.10"
import { run } from "npm:sqrl-cli@0.6.10"

export async function register(instance: Instance) {
  instance.register(
    // @todo: Add type for name once it's required
    async function sayHello(state: Execution, name) {
      return "Hello, " + name + "!";
    },
    {
      // @todo: Add argument documentation
      args: [AT.state, AT.any],
    }
  );
}

run({ register });

But, deno run -A sqrl-server.ts fails. This is expected I suppose, but what we get is:

error: npm package: sqrl@0.6.10

Caused by:
    0: error parsing version requirement for dependency: murmurhash3.js@https://github.com/qix/murmurHash3.js/releases/download/3.0.0-qix-0/murmurhash3.js-3.0.0-qix-0.tgz
    1: Invalid npm version requirement 'https://github.com/qix/murmurHash3.js/releases/download/3.0.0-qix-0/murmurhash3.js-3.0.0-qix-0.tgz'.
    2: Unexpected character.
         https://github.com/qix/murmurHash3.js/releases/download/3.0.

I think this is just because Deno doesn't support the direct https:// method for node dependencies. The actual code itself works fine, actually.

In fact, the following program does work fine, and all it does is import all the dependencies of the core sqrl library (except the murmurhash dep, but that's fine). None of these have any issues, and I don't see why the core library would either:

import * as _bigint_buffer from "npm:bigint-buffer"
import * as _bluebird from "npm:bluebird"
import * as _double_ended_queue from "npm:double-ended-queue"
import * as _eventemitter3 from "npm:eventemitter3"
import * as _fast_stable_stringify from "npm:fast-stable-stringify"
import * as _jsonschema from "npm:jsonschema"
import * as _moment from "npm:moment"
import * as _moment_timezone from "npm:moment-timezone"
import * as _node_nice from "npm:node-nice"
import * as _sqrl_common from "npm:sqrl-common"

if (import.meta.main) {
  console.log("hello world");
}

So the Murmurhash dependency seems to be the sticking point. Maybe this could be bugfixed on the Deno side but it's easy enough to workaround too, I think. There are like, 3 options here that I see:

  1. Release the murmurhash3.js package to npm, then start depending on that with a normal version constraint
  2. Migrate to a different pure-JavaScript murmurhash3 package
  3. Just inline murmurhash3.js into this package, because it's a small single file and an implementation detail anyways?

Honestly, I would vote for number 3, since it's less work to test, no need to release another new package, and just eliminates a dependency.

Note that the Redis package also depends on murmurhash3.js. Frankly I think it would be fine to just inline a copy of this file there, too; yes it's copy/paste, but I don't think it's a huge sin. Frankly you could use other hashes too here I think, so it's all a bit of a moot point — just something needs to be done.


Please note that sqrl-cli and its ilk don't necessarily need to work immediately. Deno provides its own HTTP server APIs, so really there should maybe be a sqrl-deno-http package or something. I'd actually be happy to write a simple server connecting SQRL to Deno's HTTP system. But the core library needs to work first.

thoughtpolice commented 1 year ago

There are actually many more changes needed than this for the pure TypeScript core to depend on Deno, e.g. its enforcement of isolated modules, lack of require, etc. A big one is that a local import must use a file extension (because import ... from "foo" is ambiguous as to whether it means foo.ts or foo.js.) So it's not just this. But I think it's possible to support both with some cleanups, e.g. moving all the CommonJS require imports to use ESM, etc.

So, assuming Deno support for the core lib sounds like something OK to add, I think it's probably doable if you'd accept some patches to shuffle stuff around.