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

Add support for base64 encoded buffer properties #22

Merged
merged 4 commits into from
Sep 13, 2024
Merged

Add support for base64 encoded buffer properties #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b4f221b
Add support for base64 encoded buffer properties
bhelx Sep 12, 2024
ca9daf7
Top level support
bhelx Sep 12, 2024
c34fcb3
refactor: we dont actually need this function
bhelx Sep 12, 2024
fcfe56a
bump: Bump version 15
bhelx Sep 12, 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
4 changes: 2 additions & 2 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
@@ -1,6 +1,6 @@
{
"name": "xtp-typescript-bindgen",
"version": "0.0.12",
"version": "0.0.15",
"description": "XTP TypeScript bindgen plugin",
"main": "dist/index.js",
"scripts": {
Expand Down
19 changes: 6 additions & 13 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,6 @@
import ejs from 'ejs'
import { helpers, getContext, Property, Parameter } from "@dylibso/xtp-bindgen"

function needsCasting(p: Property | Parameter): boolean {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found this ended up being redundant logic and i think i wrote it when i was experiencing a little brain fog

if (p.$ref) return true

switch (p.type) {
case "string":
if (p.format === 'date-time') return true
return false
default:
return false
}
}

function toTypeScriptType(property: Property | Parameter): string {
let tp
if (property.$ref) {
Expand Down Expand Up @@ -61,13 +49,18 @@ function toTypeScriptType(property: Property | Parameter): string {
return `${tp} | null`
}

// TODO: can move this helper up to shared library?
function isBuffer(property: Property | Parameter): boolean {
return property.type === 'buffer'
}

export function render() {
const tmpl = Host.inputString()
const ctx = {
...getContext(),
...helpers,
isBuffer,
toTypeScriptType,
needsCasting,
}
const output = ejs.render(tmpl, ctx)
Host.outputString(output)
Expand Down
8 changes: 6 additions & 2 deletions template/src/index.ts.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import {
export function <%- ex.name %>(): number {
<% if (ex.input) { -%>
<% if (isJsonEncoded(ex.input)) { -%>
<% if (isPrimitive(ex.input)) { -%>
<% if (isBuffer(ex.input)) { -%>
const input: <%- toTypeScriptType(ex.input) %> = Host.base64ToArrayBuffer(JSON.parse(Host.inputString()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Depending on if you think it'd be confusing for folks to get the ArrayBuffer handle instead of a typed array view, we may want to wrap the result of base64ToArrayBuffer in a Uint8Array.

Suggested change
const input: <%- toTypeScriptType(ex.input) %> = Host.base64ToArrayBuffer(JSON.parse(Host.inputString()))
const input: <%- toTypeScriptType(ex.input) %> = new Uint8Array(Host.base64ToArrayBuffer(JSON.parse(Host.inputString())))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, i'm not sure. input here needs to be ArrayBufferLike. wouldn't that violate it? it should be the same type as Host.inputBytes()

Copy link
Contributor

@chrisdickinson chrisdickinson Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, both Uint8Array and ArrayBuffer are ArrayBufferLike because they both match the {byteLength: number, slice(begin: number, end: number): ArrayBuffer} type.

(A useful litmus test would be whether we think users expect input[0] to return a byte or not)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's follow back up on this discussion. it feels like a broader change than just this PR. I'll file an issue.

<% } else if (isPrimitive(ex.input)) { -%>
const input: <%- toTypeScriptType(ex.input) %> = JSON.parse(Host.inputString())
<% } else { -%>
const untypedInput = JSON.parse(Host.inputString())
Expand All @@ -38,7 +40,9 @@ export function <%- ex.name %>(): number {

<% if (ex.output) { -%>
<% if (isJsonEncoded(ex.output)) { -%>
<% if (isPrimitive(ex.output)) { -%>
<% if (isBuffer(ex.output)) { -%>
Host.outputString(JSON.stringify(Host.arrayBufferToBase64(output)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto here – we can unwrap views:

Suggested change
Host.outputString(JSON.stringify(Host.arrayBufferToBase64(output)))
Host.outputString(JSON.stringify(Host.arrayBufferToBase64(ArrayBuffer.isView(output) ? output.buffer : output)))

Copy link
Contributor

Choose a reason for hiding this comment

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

(Actually, I take this back – the view has a .byteOffset that we'd need to respect by slicing the buffer!)

<% } else if (isPrimitive(ex.output)) { -%>
Host.outputString(JSON.stringify(output))
<% } else { -%>
const untypedOutput = <%- toTypeScriptType(ex.output) %>.toJson(output)
Expand Down
24 changes: 12 additions & 12 deletions template/src/pdk.ts.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ export class <%- schema.name %> {
return {
...obj,
<% schema.properties.forEach(p => { -%>
<% if (needsCasting(p)) { -%>
<% if (isDateTime(p)) { -%>
<%- p.name -%>: obj.<%- p.name -%> ? new Date(obj.<%- p.name -%>) : null,
<% } else if (!isPrimitive(p)) {-%>
<%- p.name -%>: obj.<%- p.name -%> ? <%- p.$ref.name %>.fromJson(obj.<%- p.name %>) : null,
<% } -%>
<% if (isDateTime(p)) { -%>
<%- p.name -%>: obj.<%- p.name -%> ? new Date(obj.<%- p.name -%>) : null,
<% } else if (isBuffer(p)) {-%>
<%- p.name -%>: obj.<%- p.name -%> ? Host.base64ToArrayBuffer(obj.<%- p.name -%>) : null,
<% } else if (!isPrimitive(p)) {-%>
<%- p.name -%>: obj.<%- p.name -%> ? <%- p.$ref.name %>.fromJson(obj.<%- p.name -%>) : null,
<% } -%>
<% }) -%>
}
Expand All @@ -39,12 +39,12 @@ export class <%- schema.name %> {
return {
...obj,
<% schema.properties.forEach(p => { -%>
<% if (needsCasting(p)) { -%>
<% if (p.type === "string" && p.format === "date-time") { -%>
<%- p.name -%>: obj.<%- p.name -%> ? obj.<%- p.name %>.toISOString() : null,
<% } else if (p.$ref && !p.$ref.enum) {-%>
<%- p.name -%>: obj.<%- p.name -%> ? <%- p.$ref.name %>.toJson(obj.<%- p.name %>) : null,
<% } -%>
<% if (p.type === "string" && p.format === "date-time") { -%>
<%- p.name -%>: obj.<%- p.name -%> ? obj.<%- p.name %>.toISOString() : null,
<% } else if (isBuffer(p)) {-%>
<%- p.name -%>: obj.<%- p.name -%> ? Host.arrayBufferToBase64(obj.<%- p.name -%>) : null,
<% } else if (p.$ref && !p.$ref.enum) {-%>
<%- p.name -%>: obj.<%- p.name -%> ? <%- p.$ref.name %>.toJson(obj.<%- p.name -%>) : null,
<% } -%>
<% }) -%>
}
Expand Down
16 changes: 16 additions & 0 deletions tests/schemas/fruit.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
exports:
topLevelBuffJSON:
description: Top level json buffers
input:
type: buffer
contentType: application/json
output:
type: buffer
contentType: application/json
topLevelBuffRaw:
description: Top level json buffers
input:
type: buffer
contentType: application/x-binary
output:
type: buffer
contentType: application/x-binary
voidFunc:
description: "This demonstrates how you can create an export with\nno inputs or
outputs. \n"
Expand Down
Loading