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.

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

Runtime type checking of recursive types with shield() #26

Open
louwers opened this issue Apr 22, 2022 · 6 comments
Open

Runtime type checking of recursive types with shield() #26

louwers opened this issue Apr 22, 2022 · 6 comments
Labels
enhancement ✨ New feature or request low-prio 🐌

Comments

@louwers
Copy link
Collaborator

louwers commented Apr 22, 2022

Support recursive types with shield() and auto-generate calls to shield() when using TypeScript.

Example of a recursive type:

type A = {
  a: A[]
}

More practical example: Geometry from GeoJSON.

I want to add a new shield type: t.type to which you need to pass an index, e.g. t.type(0). This is an index into an array of types that you need to pass as a third argument to shield(). For example, if a function is defined as:

export function doSomething(arg: A) {
}

The generated shield() call would be:

shield(doSomething, [t.type(0)], [{ a: t.array(t.type(0)) }]

This would be a fully backward-compatible change.

I have a proof-of-concept ready for the code generator.

While it is possible to have have a recursive arrays:

type B = B[]

I don't think those make much sense. So I will focus on object types. The approach I take is to pass a list of object types around. If a particular object type has not been seen yet, the string representation of that type needs to be generated. It can be represented as t.type(...length of list...) (before it is added to the list). If it has been seen, get the index of it in the list, and represent it as t.type(...index in list...).

@brillout
Copy link
Owner

Sounds good and makes sense 👍.

The one thing I'm worried about is the change needed to shield().

shield(doSomething, [t.type(0)], [{ a: t.array(t.type(0)) }]

The current implementation https://github.com/vikejs/telefunc/blob/9b0beae3226e5b5671d967e1186244516c3c1241/telefunc/node/server/shield.ts is quite dirty (difficult to read).

To make TypeScript inferring work, the trick is to lie to TS about the types of shield.type.*. For example, TS believes shield.type.string to be a string whereas in reality it's an object. I've tried other ways to support TypeScript inferring but they performed poorly (TS would bail out after failing at resolving types, even for types that weren't that complex). This trick works quite well; I wonder if we can use it for recursive types?

As a side note: I've been thinking of improving the error messages: https://github.com/vikejs/telefunc/blob/9b0beae3226e5b5671d967e1186244516c3c1241/telefunc/node/server/shield/shield.spec.ts#L71

- [root] > [tuple: element 0] > [object: value of key `a`] > [object: value of key `b`] > [object: value of key `c`] is `some string` but should be `42`.
+ Wrong type. Passed type: `[ { a: { b: { c: string } } } ]`. Expected type: `[ { a: { b: { c: 42 } } } ]`.

All in all, supporting recursive types would be quite neat and I'm 👍 for this, but this might require an overhaul of how shield() works.

Btw one thing we could do, if you want, is to use Vitesse for unit tests (and use Jest only for e2e tests): so that we can test the whole shield() story without Jest and, therefore, without headless browser. This will make testing shield() pretty much instantaneous (Vitesse is fast). (FYI https://vitest.dev/guide/comparisons.html.)

@louwers
Copy link
Collaborator Author

louwers commented Apr 25, 2022

@brillout The question is: how useful is type inference now that the shield() call can be auto-generated? I can play around a bit, but how would you feel about forgoing precise type inference altogether if that simplifies things?

Btw one thing we could do, if you want, is to use Vitesse for unit tests

Would love to try it out. Good idea!

@brillout
Copy link
Owner

The problem is that some stacks (e.g. React Native) do not process .telefunc.ts files. E.g. they use ts-node.

Maybe a solution would be to use ttypescript for these stacks, then we can forgoing type inference altogether (if we are confident our shield() generator to cover everything).

Example project that uses Vitesse: https://github.com/brillout/react-streaming. As you can see, it's simple to setup.

@brillout
Copy link
Owner

How often can we expect recursive types to occur?

We can also decide to support them (the user has to manually make sure his telefunction to be safe). As we wish :-).

(Btw. there are quite some exciting things in the pipeline for Telefunc, in case you are curious ;-).)

@louwers
Copy link
Collaborator Author

louwers commented Apr 25, 2022

How often can we expect recursive types to occur?

It is perfectly possible to create a (RPC) API without recursive parameters. I don't think that many APIs need them. Two examples to demonstrate their use:

  • I have an application for a project with geograpic data in mind, I would have to 'dumb down' the official GeoJSON types to work with them. (Though the RFC warns against using the recursive types when its not needed).

  • Another example: at my work we made a document editor with a GraphQL API where we also used recursive types. You can have an image or a video, which can optionally be inside a column, or inside a slideshow, or it can be wrapped in something that localizes it. (Although I think the input parameters are not recursive.)

In my view, however, Telefunc should support recursive types because they are supported by TypeScript. And within reason, I think all types whose values are serializable should be supported, because that is what I would expect with a TypeScript RPC library / framework.

The problem is that some stacks (e.g. React Native) do not process .telefunc.ts files. E.g. they use ts-node.

Is the React Native example not using telefunc.ts right now, or is it intrinsically not possible with React Native?

(Btw. there are quite some exciting things in the pipeline for Telefunc, in case you are curious ;-).)

Very! I think I'll work on a project using Telefunc soon so I have a feeling for the DX! 👍

@brillout
Copy link
Owner

Is the React Native example not using telefunc.ts right now, or is it intrinsically not possible with React Native?

This is how telefunctions are integrated on the server-side:
https://github.com/vikejs/telefunc/blob/9b0beae3226e5b5671d967e1186244516c3c1241/examples/react-native/server.mjs#L10

It could use TypeScript by using something like ts-node. But this then means that our shield() generator is not run. (ts-node doesn't support custom transformers.) So the question is how do we apply our generateShield for a React Native + Express.js stack? If we can find a solution then, yes, we can forgo shield() type inferring.

In my view, however, Telefunc should support recursive types because they are supported by TypeScript. And within reason, I think all types whose values are serializable should be supported, because that is what I would expect with a TypeScript RPC library / framework.

👍 Ok let's try.

As for Telefunc's feature pipeline, I think the most exciting short term is #27. This is quite neat as it solves a big RPC problem.

I'm almost done with the deep integration with React 18 (enabling Telefunc to fetch data for SSR apps). This means Telefunc can be used to cover all client-server communication needs. The neat thing: it's all collocated.

// TodoList.tsx
// Environemnt: Node.js & Browser

import { useTelefunc } from 'telefunc/react'
import { fetchTodoItems } from './TodoList.telefunc'

funciton TodoList() {
  const todoItems = useTelefunc(() => fetchTodoItems())
  return (
    <ul>{
      todoItems.map(item =>
        <li>{item.text}</li>
      )
    }</ul>
  )
}
// TodoList.telefunc.ts
// Environemnt: Node.js

export async function fetchTodoItems() {
  const todoItems = await query("SELECT text FROM todo_items;")
  return todoItems
}

(Normally with Next.js or vite-plugin-ssr, you need to define data fetching on an App-level, but here we define data fetching on a component-level.)

I think the same is possible for Vue, but I'll have to check.

Also short term is marketing. I've a couple of ideas for spreading Telefunc. I'm finishing the vite-plugin-ssr 0.4 release, then we can focus more on this. (I want to make progress on the docs before marketing.)

Long-term there are quite some thrilling things in the pipeline. One thing that excites me most is enabling GitHub/Facebook/... to expose public telefunctions. E.g. a "GitHub Public Telefunc Library": the idea is that GitHub provides telefunctions that are designed to be consumed by third-parties. This means a python client can seamlessly call telefunctions. This requires a cross-platform spec to be written; so let's get things right in the JS world first :-).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request low-prio 🐌
Projects
None yet
Development

No branches or pull requests

2 participants