codedownio / aeson-typescript

Generate TypeScript definition files from your ADTs
BSD 3-Clause "New" or "Revised" License
60 stars 27 forks source link

Writing custom TypeScript instances #7

Closed hasufell closed 5 years ago

hasufell commented 5 years ago

I currently don't see how to use this library with custom JSON instances, especially since the constructors for TSDeclaration are not exposed and I don't see why.

Imagine a type and the following JSON instance, which sort of "flattens":

data Foo = Foo { a :: String, b :: SomeOtherProductType }

instance ToJSON Foo where
  toJSON (Foo a b) = case toJSON b of
    (Object hm) -> Object $ HM.insert "a" (toJSON a) hm
    _ -> error "oops"

How would you write an instance for this without messing with the TypeScript instance of SomeOtherProductType? I don't see a reasonable way.

thomasjm commented 5 years ago

You can write a custom TypeScript instance where getTypeScriptType returns whatever string you like, and that string will appear on the RHS of a declaration like type Foo = [your string]. It could even make reference to the SomeOtherProductType type. Why not just write

instance TypeScript Foo where
  getTypeScriptType _ = "{a: string} & SomeOtherProductType"

I suppose if the TSDeclaration constructors were cleaned up and exposed then you would have the ability to define additional hand-written interfaces and type aliases, which might lead to better TypeScript error messages for more complicated ad-hoc types. But then you're sort of getting away from what this library is designed for, which is translating automatically derived Aeson instances to TypeScript. At that point you might as well just write out the TypeScript you want manually since this library can't help ensure correctness.

hasufell commented 5 years ago

You can write a custom TypeScript instance where getTypeScriptType returns whatever string you like, and that string will appear on the RHS of a declaration like type Foo = [your string]. It could even make reference to the SomeOtherProductType type. Why not just write

Well, this is just the type, not the declaration. The product type is huge.

At that point you might as well just write out the TypeScript you want manually since this library can't help ensure correctness.

I disagree, because I still have a complex type and I am just mixing in a single record field. Writing this interface by hand is error prone and will likely lead to bugs. I think TSDeclaration should definitely be exposed.

thomasjm commented 5 years ago

I'm not sure I want to support exposing those constructors yet, but maybe some builder functions could be exposed.

I'm having a little trouble seeing how this helps you, could you write out what you would do with the constructors if they were made public for your example?

hasufell commented 5 years ago
data Bar = Bar
  { a :: Int
  , b :: Int
  }

data Foo = Foo
  { lol  :: Lol
  , bar  :: Bar
  }

instance TypeScript Foo where
  getTypeScriptType _ = "Foo"
  getTypeScriptDeclarations _ =
    let myType = TSTypeAlternatives "Foo" [] ["IFoo"]
        fDecls = getTypeScriptDeclarations (Proxy :: Proxy Bar)
    in myType : (fmap (\x -> if interfaceName x == "IBar"
                                then (insertRecord x (TSField False "lol" (getTypeScriptType (Proxy :: Proxy Lol)))){ interfaceName = "IFoo" }
                                else x) $ Prelude.filter isDecl fDecls)
    where
      insertRecord (TSInterfaceDeclaration iname igv ifm) rec = (TSInterfaceDeclaration iname igv (rec:ifm))
      insertRecord x _ = x
      isDecl (TSInterfaceDeclaration{}) = True
      isDecl _ = False

{--

type Foo = IFoo;

interface IFoo {
  lol: Lol;
  a: number;
  b: number;
}

--}
thomasjm commented 5 years ago

See, I think what you've written here is rather hideous -- much more confusing and error-prone than simply writing out some TypeScript declarations by hand. Look at all the places where you have strings like "IBar" and "IFoo" and "lol".

This is not a library for manipulating and formatting TypeScript ASTs, which is where this goes if we start encouraging such things. If we did decide to go in that direction, the constructors would have to be more elaborate.

Also I think I got my wires crossed before--I thought there was a way to write a raw string to use as a declaration rather than a type, but there's not. What would you think about a function like

-- | Allows you to write an arbitrary declaration by hand
rawTypeScriptDeclaration :: String -> TSDeclaration

I think with this function plus the intersection type suggestion I made above you would have an elegant way to get what you want (assuming the big record type is well-behaved). You would do

myFooDeclaration = rawTypeScriptDeclaration 
  [i|type Foo = {a: string} & #{getTypeScriptType SomeOtherProductType}|]
hasufell commented 5 years ago

See, I think what you've written here is rather hideous -- much more confusing and error-prone than simply writing out some TypeScript declarations by hand.

I don't think people can be bothered to write a 150 LOC string just because the constructors are not exposed. Sure, this specific case can be handled with intersection type, but I have many more types with custom JSON instances where I might need to mess with records and I need full flexibility. The default deriving just isn't enough.

What would you think about a function like

Only solves a subset of problems.

thomasjm commented 5 years ago

How is the ability to write an arbitrary declaration as a string not "full flexibility" or "only solving a subset of problems"? It's literally the most expressive power you can have.

You should realize I came up with these constructors you're so eager to use in about 5 minutes in order to represent the small subset of TypeScript that this library can generate. They are extremely ad-hoc and that's why I'm reluctant to expose them.

If you really want TypeScript AST manipulation capabilities, I suggest you do it outside of this library. We could expose the rawTypeScriptDeclaration function I mentioned, and then you can take whatever constructors you want + some simple functions to turn them into strings and go nuts. That way the responsibility of maintaining it can be yours instead of this library's :P.

(I'm not being sarcastic, I do think that a TypeScript AST manipulation library is a reasonable thing to have, and it would be lovely if it could be used nicely with this library, but it probably doesn't belong in this library.)

hasufell commented 5 years ago

If you really want TypeScript AST manipulation capabilities, I suggest you do it outside of this library.

Alright. Seems I have to keep a fork of this library just for exposing the constructors...