nvie/decoders

Thoughts on a decoder "code generator" given a flow type.

sfarthin opened this issue · 14 comments

sfarthin commented

One pain point is the decoder is more-or-less just the flowtype rewritten. The advantage of something like https://github.com/codemix/flow-runtime is that this is done automatically, but without the flexibility of making more complex runtime decoders and/or deciding where to apply the runtime check.

What are your thoughts about a tool that would generate decoders for all exported types? For example if you have a file ./src/myModule.js and you run $ decoder-codegen ./src/, it would create a new file ./src/__generated__/myModule.decoders.js. Then we can import the decoders into the source file.

myModule.js

import { guard } from 'decoders';
import { pointDecoder } from './__generated__/myModule.decoders';

const checkPointType = guard(pointDecoder);

export type Point = {
  x: number,
  y : number
}

export function moveUp(point: Point) {
  checkPointType(point);
  point.y = point.y - 1;
}

Inspired by https://github.com/apollographql/apollo-codegen#generate

nvie commented

Since decoder definitions are typically "richer" than the Flow type, it seems a good idea to flip it around. If you define a decoder, you can always infer the type of thing it produces. But given a type, there are many decoders that can produce that type. For example:

type Book = {
  id: number,
  isbn: string,
  title: string,
};

Could at best become:

object({
  id: number,   // Negative and floats possible
  isbn: string,  // No runtime validation of ISBN
  title: string,
})

Whereas if you start from the decoder definition:

object({
  id: positiveInteger,
  isbn: regex(/^\d{9}[\d\|X]$/, 'Must be ISBN'),
  title: string,
})

It will still produce values of the type:

object({
  id: number,
  isbn: string,
  title: string,
})

But you also get the benefits of the richer syntax to describe the runtime checks you want.

Have you looked at @girvo's trick/suggestion in #93? It's a pretty solid way of extracting the Flow type from a decoder definition that might effectively be what you're looking for?

j-f1 commented

It should be possible to bundle a Babel macro with this library. That would make it possible to write something like this:

import makeGuard from 'decoders/macro'

interface Person {
  name: string
  age: number
}

const personGuard = makeGuard<Person>()

and have it generate this code:

import { guard, object, integer, string } from 'decoders'

interface Person {
  name: string
   /** @decoder integer */
  age: number
}

const _Person = object({
  name: string,
  age: integer,
})
const personGuard = guard(_Person)

Unfortunately, Babel doesn’t yet support using type parameters in function calls (makeGuard<Person>()) in Flow, but this should be able to work in TypeScript today.

The /** @decoder foo */ comments could be used to override how the field is decoded, with the value parsed by Babel and any identifiers that aren’t in scope and are exported from the decoders package could be auto-imported.

sfarthin commented

So decoders/macros would be a macro as defined here: https://github.com/kentcdodds/babel-plugin-macros ?

I believe this does work with flow (See https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVxhgGJwE5gBGeApgG4CWALgJ6q0AOJYAIiQMZwAmJeAPAGU4AWxIAVGkwB8YALxgAhgDsaAbnQ92MBaTBkdYYQoDWJAOIBXHVwBcgkeMkkZACgCUcmW049+Q0RLS6mAhYKgUSlS8UArszAAKvADOcEpgAN6oYEoKojZgSVR4EQDmYQC+6EamltZ8iXgpSlLuQA)

/* @flow */
import makeGuard from 'decoders/macro'

interface Person {
 name: string 
}

const personGuard = makeGuard<Person>()

Instead of processing comments to override how a field should be decoded, what if we had the macro determine how to decode the field by looking at imported types from the decoders lib.

import type { Integer } from 'decoders';
import makeGuard from 'decoders/macro';

interface Person {
  name: string
  age: Integer
}

const personGuard = makeGuard<Person>()
j-f1 commented

So decoders/macros would be a macro as defined here…?

That’s correct. There would be a macros.js (or macros/index.js) file in the module, and it would be run as a macro. babel-plugin-macros even has a helper function to ensure the macro is never imported in running code.

I believe this does work with flow

I checked again in the Babel REPL and you are correct! I must’ve been looking at an old version.

Instead of processing comments to override how a field should be decoded, what if we had the macro determine how to decode the field by looking at imported types from the decoders lib.

Very interesting. That would definitely be possible with the macro. Some special cases:

  • The regex decoder could be represented as Regex<Pattern, ?Flags> (i.e. Regex<'^[0-9]+$'>)
  • date could just use the native Date type.
  • null_ could just use null
  • undefined_ could just use undefined
  • constant could just use the value (i.e. "foo", 2, or true)
  • I’m not sure how hardcoded should work since it’s not necessarily a property of the type.
  • mixed/unknown can just use the equivalent literals.
  • optional, nullable, and maybe can just be represented as the literal union types. These could then be detected and converted to the appropriate calls.
  • tupleN could just use the native tuple types, but we’d need to throw if they’re longer than 6 elements or shorter than 2.
j-f1 commented

Potential API change:

import decoder from 'decoders/macro'
import { guard } from 'decoders'

interface Person {
  name: string
  age: number
}

const personGuard = guard(decoder<Person>())

This makes the macro more useful since you don’t immediately have to create a guard. The output would remain similar, with the actual declaration of the _Person decoder directly below the interface and the decoder call replaced with _Person.

sfarthin commented

Looks good, is this something you are working on @j-f1 ?

Otherwise I may take a stab at it.

j-f1 commented

Feel free to work on it!

nvie commented

Sorry for my late chipping in here. I'm not fully convinced about the need for this solution yet.

This solution adds several pieces of complexity that I would not like to let bleed into the (core) decoders library, because we'll be forever stuck with it:

  • Depending on babel-macro would mean adding build steps to use it (and knowing about how babel-macro's magic works).
  • It makes the code harder to reason about: a newcomer to decoders would perhaps not understand what's going on at runtime: it's not obvious were a runtime error is coming from. (Perhaps if they're lucky they'll recognize the importing from '.macro' and realize that code is silently generated in the background.)
  • Least important, but still a bit: even assuming the use of babel should not be necessary in the first place. (Even though babel is practically ubiquitous these days, the project was founded on the principles that one day it would become unnecessary.)

Even if we continue in the above direction, it would still require code changes from users to adopt it (and to add these special types to the decoders library):

+import type { Integer } from 'decoders';
+//            ^^^^^^^ Type with special treatment/meaning
 import makeGuard from 'decoders/macro';
 
 interface Person {
   name: string
-  age: number
+  age: Integer
 }

It might also be unclear that the type Integer will eventually become compatible with number.

Lastly, it may require to generate different code for Flow and TypeScript, and we'd have to tell babel-macro about which language to generate code for.

Alternative suggestion

If we ask users of this library to make code changes anyway, we could also ask them to do this:

// Here's a helper type we'll need. We could simply expose this in
// decoders, so this could become:
//
//     import type { ExtractDecoderType } from 'decoders');
//
type ExtractDecoderType = <T>(d: Decoder<T>) => T;

const person = object({
  name: string,
  age: number,
});

// And here the "magic" happens!
// We can extract the inferred type T from any Decoder<T> definition
type Person = $Call<ExtractDecoderType, typeof person>;

Or, if they are TypeScript users:

type Person = ExtractDecoderType<typeof person>;

This could be documented as a good practice or solution suggestion, but would not mean having to rely on any other technologies than normal JS imports and your type checker of choice.

j-f1 commented

Depending on babel-macro would mean adding build steps to use it (and knowing about how babel-macro's magic works).

Not at all! Using the macro would be entirely optional and you could still use the regular library without installing Babel (babel-plugin-macros does not depend on Babel).

It makes the code harder to reason about…

Since the macro is optional, beginners don’t have to use it. If we put some instructions in the README, they should be able to figure out they need to install Babel to use it.

Least important, but still a bit: even assuming the use of babel should not be necessary in the first place. (Even though babel is practically ubiquitous these days, the project was founded on the principles that one day it would become unnecessary.)

I think that principle was from before ECMA started updating JS annually. Now, there’ll always be syntax features that haven’t made it into JS yet, and Babel’s become a tool for doing things just like this.

The reason I made the API go from type to value is that people adopting decoders will already have type definitions, and it’s easier to insert one line of code to get a decoder from the type definition than to rewrite your types, then convert them back to a type.

sfarthin commented
sfarthin commented

fyi, proof of concept started: https://github.com/sfarthin/flow-guard . Might be some time translating the flow AST into decoder code. (https://github.com/sfarthin/flow-guard/blob/master/macro.js#L5)

  • 👍1
j-f1 commented

Why did you decide on a guard API rather than a decoder API?

sfarthin commented

Guard satisfies the majority of my use cases and does not require understanding of decoders to use, though it would be nice in certain situations.

The lib could provide both:

import guard from "flow-guard/macro";
import createDecoder from "flow-guard/decoder-macro";

Additionally, I am thinking decoders that go beyond flow types (integer, regex, email, etc), it might make more sense to have that as an additional optional argument that would "extend" the decoder. The implementation of pulling this from comments or somewhere else would be tricky and maybe a little too magical.

import { email } from 'decoders';
import guard from "flow-guard/macro";

type Person = {
  name: string,
  email: string
}

function personGuard(person: any): Person {
  return guard<Person>(person, {
    email
  });
}

or with decoders...

import { guard, email } from 'decoders';
import createDecoder from "flow-guard/decoder-macro";

type Person = {
  name: string,
  email: string
}

const personDecoder = createDecoder<Person>({
  email
});

const personGuard = guard(personDecoder);
  • 👍2
github-actions commented

We have detected this issue has not had any activity during the last 60 days. That could mean this issue is no longer relevant and/or nobody has found the necessary time to address the issue. We are trying to keep the list of open issues limited to those issues that are relevant to the majority and to close the ones that have become 'stale' (inactive). If no further activity is detected within the next 14 days, the issue will be closed automatically.
If new comments are are posted and/or a solution (pull request) is submitted for review that references this issue, the issue will not be closed. Closed issues can be reopened at any time in the future. Please remember those participating in this open source project are volunteers trying to help others and creating a better collaboration environment for all. Thank you for your continued involvement and contributions!