-
Notifications
You must be signed in to change notification settings - Fork 65
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
WIP: Improve and simplify Deno compatibility #632
base: master
Are you sure you want to change the base?
Conversation
Started doing this after discussion in #599, please let me know what you think so far, especially if it looks like I'm unknowingly about to break compatibility with something important. |
I should add that the reason why I've added |
TSError: src/baseConn.ts:533:9 - error TS2454: Variable 'inTypeId' is used before being assigned. src/baseConn.ts:533:29 - error TS2454: Variable 'outTypeId' is used before being assigned. src/baseConn.ts:619:9 - error TS2454: Variable 'cardinality' is used before being assigned.
b718b86
to
2a22a5e
Compare
Not sure why this wasn't done in the first place?
This appeases `deno lint`
This helps Denoify
Use yarn run instead of npx
2a22a5e
to
dd2591d
Compare
Work In Progress – I'm aiming to remove all "custom" Deno build logic here in favor of Denoify.
packages/driver
packages/generate
???
I've taken some liberties with things like changing file naming conventions in favor of Denoify's default pattern, where
foobar.deno.ts
is the Deno replacement forfoobar.ts
. Previously, it's beenfoobar.node.ts
andfoobar.deno.ts
explicitly.A bunch of minor cosmetic changes to appease the Deno linter.
Ended up removing
driver/.../globals.deno.ts
because I couldn't figure out what it was for...?Explicitly import Node.js builtins, e.g.
import { Buffer } from "buffer"
because Denoify needs it to do its thing. Ideally, I'd want to usenode:buffer
style imports, Denoify actually turns that intonode:node:buffer
, so I'm holding off on that.