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.

Sign up for GitHub

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

Jump to bottom

Refactor Bindgen #37

Merged
merged 3 commits into from
Nov 11, 2024
Merged

Refactor Bindgen #37

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e85fcf
Refactor Bindgen
bhelx Oct 25, 2024
e983162
get XtpTyped from the project
bhelx Nov 1, 2024
cbc1caf
bump to latest
bhelx Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"typescript": "^5.3.2"
},
"dependencies": {
"@dylibso/xtp-bindgen": "1.0.0-rc.11",
"@dylibso/xtp-bindgen": "1.0.0-rc.13",
"ejs": "^3.1.10"
}
}
139 changes: 90 additions & 49 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,65 +1,106 @@
import ejs from 'ejs'
import { helpers, getContext, Property, Parameter } from "@dylibso/xtp-bindgen"
import { helpers, getContext, ObjectType, EnumType, ArrayType, XtpNormalizedType, MapType, XtpTyped } from "@dylibso/xtp-bindgen"

function toTypeScriptType(property: Property | Parameter): string {
let tp
if (property.$ref) {
tp = property.$ref.name
} else {
switch (property.type) {
case "string":
if (property.format === 'date-time') {
tp = 'Date'
} else {
tp = "string"
}
break
case "integer":
if (property.format === 'int64') {
throw Error(`We do not support format int64 yet`)
} else {
tp = "number"
}
break
case "number":
tp = "number"
break
case "boolean":
tp = "boolean"
break
case "object":
tp = "any"
break
case "array":
if (!property.items) {
tp = 'Array<any>'
} else {
// TODO this is not quite right to force cast
tp = `Array<${toTypeScriptType(property.items as Property)}>`
}
break
case "buffer":
tp = "ArrayBufferLike"
break
}
function toTypeScriptTypeX(type: XtpNormalizedType): string {
// annotate with null if nullable
const nullify = (t: string) => `${t}${type.nullable ? ' | null' : ''}`

switch (type.kind) {
case 'string':
return nullify('string')
case 'int32':
case 'float':
case 'double':
case 'byte':
return nullify('number')
case 'date-time':
return nullify('Date')
case 'boolean':
return nullify('boolean')
case 'array':
const arrayType = type as ArrayType
return nullify(`Array<${toTypeScriptTypeX(arrayType.elementType)}>`)
case 'buffer':
return nullify('ArrayBufferLike')
case 'object':
const oType = (type as ObjectType)
if (oType.properties?.length > 0) {
return nullify(oType.name)
} else {
return nullify('any')
}
case 'enum':
return nullify((type as EnumType).name)
case 'map':
const { keyType, valueType } = type as MapType
return nullify(`Record<${toTypeScriptTypeX(keyType)}, ${toTypeScriptTypeX(valueType)}>`)
case 'int64':
throw Error(`We do not support format int64 yet`)
Comment on lines +37 to +38
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may want to make this a bigint?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, bigint would cover it!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to think a bit about how we should encode it since i think we're limited by the JSON spec. maybe just as a numerical string? "123456789098765432"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how (at least in Chrome) throws:

Do not know how to serialize a BigInt

😆

default:
throw new Error("Cant convert property to typescript type: " + JSON.stringify(type))
}
}

if (!tp) throw new Error("Cant convert property to typescript type: " + property.type)
if (!property.nullable) return tp
return `${tp} | null`
function toTypeScriptType(property: XtpTyped): string {
return toTypeScriptTypeX(property.xtpType!)
}

// TODO: can move this helper up to shared library?
function isBuffer(property: Property | Parameter): boolean {
return property.type === 'buffer'
/**
* Check whether this type needs to be cast or not
*/
function isCastable(t: XtpNormalizedType): boolean {
if (['date-time', 'buffer'].includes(t.kind)) return true

switch (t.kind) {
case 'object':
const oType = t as ObjectType
// only return true when the object has defined / typed properties
return (oType.properties && oType.properties.length > 0)
case 'array':
return isCastable((t as ArrayType).elementType)
case 'map':
return isCastable((t as MapType).valueType)
default:
return false
}
}

/**
* Renders the function call to cast the value
* Assumes the target is called `obj`
*
* Example: Assume we have a map of arrays of dates
* castExpression(t, 'From') would yield:
* -> cast(castMap(castArray(dateFromJson)), obj.myObj)
*
* castExpression(t, 'To') would yield:
* -> cast(castMap(castArray(dateToJson)), obj.myObj)
*/
function castExpression(t: XtpNormalizedType, direction: 'From' | 'To'): string {
switch (t.kind) {
case 'object':
const oType = t as ObjectType
return `${oType.name}.${direction.toLowerCase()}Json`
case 'array':
return `castArray(${castExpression((t as ArrayType).elementType, direction)})`
case 'map':
return `castMap(${castExpression((t as MapType).valueType, direction)})`
case 'date-time':
return `date${direction}Json`
case 'buffer':
return `buffer${direction}Json`
default:
throw new Error(`Type not meant to be casted ${JSON.stringify(t)}`)
}
}

export function render() {
const tmpl = Host.inputString()
const ctx = {
...getContext(),
...helpers,
isBuffer,
isCastable,
castExpression,
toTypeScriptType,
}
const output = ejs.render(tmpl, ctx)
Expand Down
12 changes: 6 additions & 6 deletions template/src/index.ts.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ export function <%- ex.name %>(): number {
<% if (isJsonEncoded(ex.input)) { -%>
<% if (isBuffer(ex.input)) { -%>
const input: <%- toTypeScriptType(ex.input) %> = Host.base64ToArrayBuffer(JSON.parse(Host.inputString()))
<% } else if (isPrimitive(ex.input)) { -%>
const input: <%- toTypeScriptType(ex.input) %> = JSON.parse(Host.inputString())
<% } else { -%>
<% } else if (isObject(ex.input)) { -%>
const untypedInput = JSON.parse(Host.inputString())
const input = <%- toTypeScriptType(ex.input) %>.fromJson(untypedInput)
<% } else { -%>
const input: <%- toTypeScriptType(ex.input) %> = JSON.parse(Host.inputString())
<% } -%>
<% } else if (ex.input.type === 'string') { -%>
const input = Host.inputString() <%- (ex.input.$ref && ex.input.$ref.enum) ? `as ${ex.input.$ref.name}` : "" %>
Expand All @@ -42,11 +42,11 @@ export function <%- ex.name %>(): number {
<% if (isJsonEncoded(ex.output)) { -%>
<% if (isBuffer(ex.output)) { -%>
Host.outputString(JSON.stringify(Host.arrayBufferToBase64(output)))
<% } else if (isPrimitive(ex.output)) { -%>
Host.outputString(JSON.stringify(output))
<% } else { -%>
<% } else if (isObject(ex.output)) { -%>
const untypedOutput = <%- toTypeScriptType(ex.output) %>.toJson(output)
Host.outputString(JSON.stringify(untypedOutput))
<% } else { -%>
Host.outputString(JSON.stringify(output))
<% } -%>
<% } else if (ex.output.type === 'string') { -%>
Host.outputString(output)
Expand Down
80 changes: 47 additions & 33 deletions template/src/pdk.ts.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,45 @@ function isNull(v: any): boolean {

function cast(caster: (v: any) => any, v: any): any {
if (isNull(v)) return v
if (Array.isArray(v)) return v.map(caster)
return caster(v)
}

function dateToJson(v: Date): string {
function castArray(caster: (v: any) => any) {
return (v?: Array<any>) => {
if (isNull(v)) return v
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A (kind-of lengthy) question! So, IIUC right now the JS bindgen is more permissive than other bindgens about what values it accepts. That can lead to some surprise down the line for hosts if they build their host service and initial plugins using JS. (This happened in extendabot: some interfaces were actually nullable but not marked as such. The JS bindgen plugins worked, but go, rust, and others failed in ways that were difficult to debug from the guest side.)

Should we track type nullability here and fail if an array (or array member) is set to null inappropriately to match the other bindgens? (It's possible we're doing that and I just missed it, too!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of comes at the question of the line b/w types and validation. Who is responsible for validation and where should that happen? I wanted this casting code to be agnostic to that. Though I understand with other, typed languages you get more validation out of the box. I still need to think a bit about where they should happen. We could maybe follow up with a stricter casting routine. I'm still holding out hope that we can use a dependency for all this code and delete it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something to note about the casting code if it's not clear, we don't cast the whole object. only the parts that need it. So i do think if we consider some kind of validation we may need to do that on the whole object somehow.

caster = cast.bind(null, caster) // bind to null-preserving logic in `cast`
return v!.map(caster)
}
}

function castMap(caster: (v: any) => any) {
return (v?: any) => {
if (isNull(v)) return v

caster = cast.bind(null, caster) // bind to null-preserving logic in `cast`
const newMap: any = {}
for (const k in v) {
newMap[k] = caster(v![k])
}
return newMap
}
}

function dateToJson(v?: Date): string | undefined | null {
if (v === undefined || v === null) return v
return v.toISOString()
}
function dateFromJson(v: string): Date {
function dateFromJson(v?: string): Date | undefined | null {
if (v === undefined || v === null) return v
return new Date(v)
}

function bufferToJson(v: ArrayBuffer): string {
function bufferToJson(v?: ArrayBuffer): string | undefined | null {
if (v === undefined || v === null) return v
return Host.arrayBufferToBase64(v)
}
function bufferFromJson(v: string): ArrayBuffer {
function bufferFromJson(v?: string): ArrayBuffer | undefined | null {
if (v === undefined || v === null) return v
return Host.base64ToArrayBuffer(v)
}

Expand All @@ -48,14 +72,8 @@ export class <%- schema.name %> {
return {
...obj,
<% schema.properties.forEach(p => { -%>
<% let baseP = p.items ? p.items : p -%>
<% let baseRef = p.$ref ? p.$ref.name : (p.items && p.items.$ref ? p.items.$ref.name : null) -%>
<% if (isDateTime(baseP)) { -%>
<%- p.name -%>: cast(dateFromJson, obj.<%- p.name -%>),
<% } else if (isBuffer(baseP)) {-%>
<%- p.name -%>: cast(bufferFromJson, obj.<%- p.name -%>),
<% } else if (!isPrimitive(baseP)) {-%>
<%- p.name -%>: cast(<%- baseRef -%>.fromJson, obj.<%- p.name -%>),
<% if (isCastable(p.xtpType)) { -%>
<%- p.name -%>: cast(<%- castExpression(p.xtpType, 'From') %>, obj.<%- p.name %>),
<% } -%>
<% }) -%>
}
Expand All @@ -65,26 +83,20 @@ export class <%- schema.name %> {
return {
...obj,
<% schema.properties.forEach(p => { -%>
<% let baseP = p.items ? p.items : p -%>
<% let baseRef = p.$ref ? p.$ref.name : (p.items && p.items.$ref ? p.items.$ref.name : null) -%>
<% if (isDateTime(baseP)) { -%>
<%- p.name -%>: cast(dateToJson, obj.<%- p.name -%>),
<% } else if (isBuffer(baseP)) {-%>
<%- p.name -%>: cast(bufferToJson, obj.<%- p.name -%>),
<% } else if (!isPrimitive(baseP)) {-%>
<%- p.name -%>: cast(<%- baseRef -%>.toJson, obj.<%- p.name -%>),
<% if (isCastable(p.xtpType)) { -%>
<%- p.name -%>: cast(<%- castExpression(p.xtpType, 'To') %>, obj.<%- p.name %>),
<% } -%>
<% }) -%>
}
}
}
<% } else if (schema.enum) { %>
<% } else if (isEnum(schema)) { %>

/**
* <%- formatCommentLine(schema.description) %>
*/
export enum <%- schema.name %> {
<% schema.enum.forEach(variant => { -%>
<% schema.xtpType.values.forEach(variant => { -%>
<%- variant
// "host:foo$bar" -> "host_Foo$bar"
.replace(/[^a-zA-Z0-9_$]+(.)?/g, (a, m) => '_' + (m ||'').toUpperCase())
Expand Down Expand Up @@ -115,15 +127,16 @@ export enum <%- schema.name %> {
export function <%- imp.name %>(<%- imp.input ? `input: ${toTypeScriptType(imp.input)}` : null %>) <%- imp.output ? `:${toTypeScriptType(imp.output)}` : null %> {
<% if (imp.input) { -%>
<% if (isJsonEncoded(imp.input)) { -%>
<% if (isPrimitive(imp.input)) { %>
const mem = Memory.fromJsonObject(input as any)
<% } else { %>
const casted = <%- toTypeScriptType(imp.input) %>.toJson(input)
<% if (isObject(imp.input)) { %>
// we need to cast the input back into a json encodable form
const casted = cast(<%- castExpression(imp.input.xtpType, 'To') %>, input)
const mem = Memory.fromJsonObject(casted)
<% } else { %>
const mem = Memory.fromJsonObject(input as any)
<% } %>
<% } else if (isUtf8Encoded(imp.input)) { -%>
const mem = Memory.fromString(input as string)
<% } else if (imp.input.type === 'string') { -%>
<% } else if (isString(imp.input.type)) { -%>
const mem = Memory.fromString(input)
<% } else { -%>
const mem = Memory.fromBuffer(input)
Expand All @@ -136,15 +149,16 @@ export function <%- imp.name %>(<%- imp.input ? `input: ${toTypeScriptType(imp.i

<% if (imp.output) { -%>
<% if (isJsonEncoded(imp.output)) { -%>
<% if (isPrimitive(imp.output)) { -%>
return Memory.find(ptr).readJsonObject();
<% } else { -%>
<% if (isObject(imp.output)) { -%>
// we need to cast the output back from its json encodable form
const output = Memory.find(ptr).readJsonObject();
return <%- toTypeScriptType(imp.output) %>.fromJson(output)
return cast(<%- castExpression(imp.output.xtpType, 'From') %>, output)
<% } else { -%>
return Memory.find(ptr).readJsonObject();
<% } -%>
<% } else if (isUtf8Encoded(imp.output)) { -%>
return Memory.find(ptr).readString();
<% } else if (imp.output.type === 'string') { -%>
<% } else if (isString(imp.output)) { -%>
return Memory.find(ptr).readString();
<% } else { -%>
return Memory.find(ptr).readBytes();
Expand Down
Loading
Loading