Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

Stop using auto-generated files #75

Open
dbrattli opened this issue Jan 9, 2019 · 16 comments
Open

Stop using auto-generated files #75

dbrattli opened this issue Jan 9, 2019 · 16 comments

Comments

@dbrattli
Copy link

dbrattli commented Jan 9, 2019

Working with the current bindings is not a good experience for developers. The ts2fable generated code is hard to read (recursive modules), missing imports, and generally a pain to use making developers spend hours and hours struggling before giving up or re-implement the binding manually themselves. This is hurting both the Fable community, and the idea of embracing the JS community. No one wants to patch auto-generated files so developers makes fixes for themselves that doesn't help others. Using ts2fable to generate the initial bindings is ok, but from then on it should be maintained manually (imo). So comments like // ts2fable 0.5.2 should be removed so developers get encouraged to work on them manually and improve them.

@MangelMaxime
Copy link
Member

MangelMaxime commented Jan 9, 2019

I think there is several things in your comment :)

The ts2fable generated code is hard to read (recursive modules)

Main problem here is that we are mapping a dynamic language where everything is allowed to a strongly typed and opinated language. For example, declaration order matter in F# where not in JavaScript.

Using recursive module is the solution to solve order declaration problem and even with that we can't support all the JavaScript eco-system.

missing imports

Then, we should improve ts2fable to include them.

generally a pain to use making developers spend hours and hours struggling before giving up or re-implement the binding manually themselves

Here I think it highlights severals things:

  • We are missing example usage of the binding, so people know how to use them
  • Most people don't understand Fable/JavaScript interop so they don't how an interface map to the JavaScript API

I saw a lot of people creating bindings by replacing the type (class/interface) with module/function. So basically they were doing a one to one mapping to the JavaScript and couldn't port everyting.

No one wants to patch auto-generated files so developers makes fixes for themselves that doesn't help others

I tend to think that people makes fixes for themselve because it's easier. For example, several bindings in this repo don't contains information that they were auto generated and even so they didn't receive much more love. Perhaps, people don't use them it's also a possibility.

I think, that people tend to not send fixes, because it needs several steps:

  • clone the repo
  • apply the fix
  • write a PR
  • wait for a Release
  • Remove their local fix to use the original library

Using ts2fable to generate the initial bindings is ok, but from then on it should be maintained manually (imo)

Yes and no. We can't say from the time the binding has been generated once, we should never generate it again.

In general, we generate the binding, add improvement by hand to them. Like converting float to int where it make sense, etc. But when the library got a massive update, you most likely will need to generate the binding again to have all the new types. So the hard part is to make improvement while staying close enough to the original definitation so git diffing can help you update the binding definition while keeping the improvements made.

So comments like // ts2fable 0.5.2 should be removed so developers get encouraged to work on them manually and improve them.

If you think it can help, sure we can remove these commands from the library in this repo. :)


In general, bindings in this repo are first generated and improved over time by hand as I described it. Which is in fact what you said :)

@dbrattli
Copy link
Author

dbrattli commented Jan 9, 2019

Main problem here is that we are mapping a dynamic language where everything is allowed to a strongly typed and opinated language. For example, declaration order matter in F# where not in JavaScript.

Yes, TypeScript and JS does not necessarily map well to F#, which is why auto-generating the bindings might not be the right thing to do.

Using recursive module is the solution to solve order declaration problem and even with that we can't support all the JavaScript eco-system.

Most modules could probably be written without being recursive. Now it looks like every generated module is recursive. It confuses users and it confuses Ionide. The same goes for with Array vs ResizeArray, as you end up with messy code that mixes and converts to and from Array, ResizeArray, and list. There's probably a good reason for this, I'm just saying that the experience is not good.

Then, we should improve ts2fable to include them.

Yes, but as you say, the JS ecosystem is so complex so ts2fable will always be 10 steps behind. Getting users to send fixes for bindings might be hard, but require them to submit fixes for ts2fable is much harder.

I saw a lot of people creating bindings by replacing the type (class/interface) with module/function. So basically they were doing a one to one mapping to the JavaScript and couldn't port everyting.

Yes, what happens now is that you give up on the bindings and write the few functions you need yourself.

In general, we generate the binding, add improvement by hand to them. Like converting float to int where it make sense, etc. But when the library got a massive update, you most likely will need to generate the binding again to have all the new types. So the hard part is to make improvement while staying close enough to the original definitation so git diffing can help you update the binding definition while keeping the improvements made.

If you need to regenerate the binding because of massive updates (e.g version 0 to version 1), then version 0 should first be put in a v0 directory before generating a new one (e.g using ts2fable).

If you think it can help, sure we can remove these commands from the library in this repo. :)

In general, bindings in this repo are first generated and improved over time by hand as I described it. Which is in fact what you said :)

I think it would help a lot to remove the comment, add a README.md for usage information for each library, and encourage developers to contribute and improve the bindings when they find missing pieces, and also contribute to the usage information. That is the only way to move forward and improve the experience of using Fable beyond the SAFE template and Fulma. Ts2fable is a great tool, but I think we are expecting too much, and using it the wrong way.

@MangelMaxime
Copy link
Member

Before answering, I just want to say thank you for taking time to discuss on this issue. And I want to say, I am not against anything you say. I am discussing and explaining things so we can find a common ground and perhaps result in a list of action to do :)


If you need to regenerate the binding because of massive updates (e.g version 0 to version 1), then version 0 should first be put in a v0 directory before generating a new one (e.g using ts2fable).

This is indeed a good idea but the migration process I mentioned is still valid otherwise you would have potential regressions.
Also this would need to adapt the release process as for now, it only understand one "active" version per folder.

Yes, TypeScript and JS does not necessarily map well to F#, which is why auto-generating the bindings might not be the right thing to do.

I guess we can agree on that point. Auto generation is probably hard or impossible to support but we need to use it to do 90% of the job.
Otherwise, people would need to do the job I do for Fulma where I am writing everything by hand. And it's not a sustainable way for JS bindings if we can avoid it :)

Using recursive module is the solution to solve order declaration problem and even with that we can't support all the JavaScript eco-system.

Most modules could probably be written without being recursive. Now it looks like every generated module is recursive. It confuses users and it confuses Ionide. The same goes for with Array vs ResizeArray, as you end up with messy code that mixes and converts to and from Array, ResizeArray, and list. There's probably a good reason for this, I'm just saying that the experience is not good.

I don't know, I remember times where we didn't have the recursive module and to be honest it was a lot of trial and error to fix the order declaration.

About Ionide going crazy, if we can avoid it that's nice but that an F# feature so it should support it :)

From my experience, the problem is more that there was a lot of line to parse than the recursives modules.

For example, you can try editing Fulma library and documentation site in Ionide and you will see it freeze, etc.

As a maintainer the recursive modules simplify the process of maintaining a library because files between 2 generation from ts2fable looks the same and so I can apply the correct change and still keep the manaul improvements made.

That's something impossible if you re-order the file manually. If you don't re-order the declaration in the exact same order then the diff will not help you and you will probably miss porting manual improvement made.

I saw a lot of people creating bindings by replacing the type (class/interface) with module/function. So basically they were doing a one to one mapping to the JavaScript and couldn't port everyting.

Yes, what happens now is that you give up on the bindings and write the few functions you need yourself.

Well, if people does that then they are just fixing their today problem and not the one from tomorrow. I mean when I am porting a library I always port all of it because I never know what I will need and I don't want to write binding all the day. I prefer to do it once and be done with it :)

But this point is probably out of the subject here. It's just a side effect of all the others point you mention :)

If you think it can help, sure we can remove these commands from the library in this repo. :)
In general, bindings in this repo are first generated and improved over time by hand as I described it. Which is in fact what you said :)

I think it would help a lot to remove the comment, add a README.md for usage information for each library, and encourage developers to contribute and improve the bindings when they find missing pieces, and also contribute to the usage information. That is the only way to move forward and improve the experience of using Fable beyond the SAFE template and Fulma. Ts2fable is a great tool, but I think we are expecting too much, and using it the wrong way.

  • Removing the comment is easy
  • Adding README.md for usage information about the library is probably the minimal action to do but it's hard to keep it update with the code. I mean because it's not part of an actual F# project, we don't have compiler error etc. Best things would be to have installation instruction in the README.md and a samples directory so we are sure it's up to date with the binding code ?

Ts2fable is a great tool, but I think we are expecting too much, and using it the wrong way.

Well people tend to like auto/magic stuff and will always. I know that as people love auto decoders when I "hate" them :)

Perhaps, this is up to us to educate people and explain how ts2fable is meant to be used. Have good explanation of the issues, limitations, etc.

@alfonsogarciacaro
Copy link
Member

Sorry, I haven't read the full thread yet but you're right @dbrattli. The dream of having mostly automatic bindings with very little maintenance is going away with each new release of Typescript as the type system diverges more a more from F#.

Giving up automatic translation with ts2fable (or better, restricting it to the first draft) goes also in line with the discussion in #73 to split the browser bindings in different Web APIs. At the end we'll probably have to deprecate this, have each binding in its own repo and ask for volunteers to maintain them when possible (moving the repos if necessary) as we've done with Fable.PowerPack. Then, we just need a page in Fable's website that allows user to search for available packages/bindings, as discussed in fable-compiler/fable-compiler.github.io#32

@dbrattli
Copy link
Author

Ok, @alfonsogarciacaro that sounds like a great idea!

It might still be good to have the separate repos under fable-compiler but delegate the maintenance to Collaborators with write access to the given repo only. That way it's easier for someone else to take over in case the repo gets abandoned. That's better than having external repos that may easier die out and other will only have the option to fork to continue.

I'm going to work on Leaflet, ReactLeaflet and GeoJSON in the months ahead, so if you split those into separate repos I would be happy to contribute.

@cmeeren
Copy link

cmeeren commented Feb 3, 2019

I duplicated this in #78 because I didn't search for the right keywords. Closing that issue and re-posting here with some modifications.

I completely agree with @dbrattli. Auto-generated bindings, such as the Electron bindings, are not particularly F# idiomatic and can cause high friction. This goes for type safety as well as other stuff (e.g. avoiding explicit callbacks in favour of async/ promise).

A couple of concrete examples I came across:

  • When using electron.remote.dialog.showOpenDialog, it returns a ResizeArray<string> that is undefined if the user cancelled the dialog. The most F# idiomatic API here would be to return string list option or string array option, or at the very least a ResizeArray<string> option if it has to be ResizeArray for performance reasons.

  • fs.readFile reads a file asynchronously and takes an explicit continuation callback. In F# it is more natural to use a CE (async/promise). The callback is also not that type safe; it accepts two parameters, error and string/buffer (depending on overload), and AFAIK only if the error is None should you use the other parameter (similar to e.g. Int32.TryParse). Result would have been better.

  • Some cases of obj could be replaced with new types. For example, the property OpenDialogOptions.filters (in the Electron bindings) is of type ResizeArray<obj> option. According to the showOpenDialog documentation, this property is an array of FileFilter which is a simple and well defined type.

  • Some cases of string could be replaced with a StringEnum DU. For example, the property OpenDialogOptions.properties is of type ResizeArray<string> option, but only supports a predefined set of values, making a StringEnum suitable.

  • The APIs are accessed through global objects (electron.remote.dialog.showOpenDialog). In F# it would be more natural to access them through similarly named modules (Electron.Remote.Dialog.ShowOpenDialog), One can argue that it's best keeping this as close to the official Electron API (and documentation) as possible, but on the other hand it often causes me some inconvenience trying to find the right object to "enter" the graph (e.g. electron, which is defined in Fable.Import.electron_Extensions, which puzzled me somewhat). It might also be the case that it's simply not possible in many cases to convert the API from object-based to module-based. But in many cases it's likely possible.

Fable does not hide the fact that it compiles to JS. This is both a great strength (it's fairly easy to interact with unsupported JS APIs, and devs have to learn a bit about the JS runtime environment anyway) and a significant weakness (non-trivial code needing several JS APIs, even supported ones through bindings such as in this project, may end up looking in part like JS code written with a weird and limiting syntax, and can cause troubles when functions can return e.g. undefined unannounced through the type system). I think we all agree that type safety (cf. the first bullet point) is important and should be ensured. I think most of you also agree that it's both safer, easier and more F# idiomatic to use list (or even array since it too can be pattern-matched) instead of ResizeArray if performance allows. (This might necessitate wrapping the relevant JS bindings in a function that performs the conversion, though I'm a bit puzzled by Fable's collection types, so for all I know some "conversions" might be just a matter of casting.) Personally, I'd like all APIs to "feel" like .NET and not like JS (F# is a .NET language, after all), but I can see "API feel" being a source of differing opinions.

I'm therefore of the clear opinion that API/bindings in general should be more F# idiomatic. The bindings should not only allow calling the JS API in the first place; the JS API should (where performance allows) be wrapped and presented in a form that causes low friction from F#.

I might be able to assist with this, but I'd need clear guidelines on what kinds of things are desirable and acceptable to change, and it would likely progress slowly due to my limited capacity in the foreseeable future. I can also make no promises as to how committed I can be to cleaning up everything (as opposed to just a few bits here and there to scratch my own itches), or whether I will be able to continually maintain an F# idiomatic API as new bindings are needed for new JS API surfaces.

Note though that I think there are a lot of API changes that can be done and benefited from in isolation, so there is no strict need to do this "all at once" (except for the fact that all changes would likely be breaking, and it might be desirable to update as much as possible at once to avoid many major releases).

@MangelMaxime
Copy link
Member

The thing is that in order to make the API feel more .Net you would have to write a lot of it by hand. And check the documentation in order to see all the possible case and then create the correct bindings.

For point 1, we use ResizeArray because it's the most generic type. If you use array Fable will generate optimized array like IntArray, Float32Array or things like that.

For the point 2, we are binding Electron API. If you want to use promise it's the same as in JavaScript you have to write a helper. And we could publish it in the package just like we include react helpers with the bindings in Fable.React.

For point 3, if we generated obj it's probably because either ts2fable wasn't able to understand the code or the definition wasn't complete.

For point 4, we kept the same name as in JavaScript so in general, the F# code is the same as in JavaScript and people can then benefit from JavaScript documentation, SO answers, etc.


For me, point 1, 2, 3 are the same. As with all the bindings, we generated the code using ts2fable and then it needs tweaking by hand. I mean, imagine having the d.ts and electron doc site only. I bet no one would be able to write the full binding by hand.

If you want and if others are ok, you can indeed propose a PR to the electron binding in order to make it more friendly.

@cmeeren
Copy link

cmeeren commented Feb 3, 2019

The thing is that in order to make the API feel more .Net you would have to write a lot of it by hand. And check the documentation in order to see all the possible case and then create the correct bindings.

Yes, that's exactly the point. Given the current state of auto-generated bindings, I think hand-crafted bindings could considerably improve the usability of the bindings and make Fable even more attractive and easy to use. It would also make it possible to include function/parameter documentation in cases where the TS bindings don't include them (it seems to be missing in a lot of places). It does leads to significantly more maintenance, though.

For point 1, we use ResizeArray because it's the most generic type. If you use array Fable will generate optimized array like IntArray, Float32Array or things like that.

Is that a bad thing? In any case: When creating hand-crafted bindings, how to I know whether to prefer ResizeArray, array, list, or seq?

I mean, imagine having the d.ts and electron doc site only. I bet no one would be able to write the full binding by hand.

Sorry, I didn't understand this. Aren't docs alone sufficient to create bindings? (Assuming the docs cover the complete API.)

@MangelMaxime
Copy link
Member

Sorry, I didn't understand this. Aren't docs alone sufficient to create bindings? (Assuming the docs cover the complete API.)

Yes, docs are sufficient or even the source code of the library. I did it for Fulma for example and it took me 3-4 months of work to port the whole API.

And Fulma was "simple" because it was just CSS wrapper. In what you propose, when porting the API it's not only making the bindings but also all the helpers for the functions that are using callbacks. But in this case, should we make the helpers ourself or use an existing JavaScript library who is already converting to callback and so make the bindings for it and so on etc.

But I can't imagine someone crazy enough doing it an API like electron or worst VSCode. But I would be really happy to be proven wrong 😉.

I am kind of playing a bit the advocate of devil here to help identify the pain point. And be able as a community to weight the pros and cons and also consider the amount of work to be done.

@drk-mtr
Copy link

drk-mtr commented Feb 20, 2019

I have the same experience unfortunately. No easy solution really, the state of community maintained bindings will be a function of how active the community is - so I guess we just keep telling people how great it is!

I've tried to use ts2fable on a few libraries and have had some very limited success stripping out just the types that I need. Generally though, the whole thing falls apart when I start trying to change things and I'm not knowledgeable enough with JS or F# to know how to fix anything. And I can't use the file itself because Ionide grinds to a halt.

So my usual experience is: get a working javascript example up and running in about a minute - then try to port it and spend 5 hours hacking away with types and end up with a spaghetti of red squiggles, then give up. I'm sure it would be an incredible time saving tool for a more experienced developer though.

For those of us who are doing this to try and avoid JS, we end up getting stubborn and really wanting types rather than falling back to dynamic ha. I think if I felt more comfortable with interop generally this probably wouldn't be such a pain point.

At the moment I feel poring through other (non generated) bindings (and resources like this whitetigle article on medium), learning how they do things, then applying the knowledge to just pluck the types I need out is probably the way forward.

@MangelMaxime
Copy link
Member

Hello @drk-mtr ,

thank you for sharing your experience.

Writing a binding indeed isn't an easy task. And need the developer to learn how JavaScript data transpose to F# types. Did you found this interop guide. It's a great learning resource and almost complete.

Would you say we are missing documentation and examples on how to write bindings? How to structure it?

I did write several bindings in the past weeks and was wondering if sharing my experience and how I did it as @whitetigle did would help or no.

@whitetigle
Copy link
Contributor

@MangelMaxime, I think sharing your experience would definitely help because today the main point is: how can we convince people to use fable if they can't use their favorite libs?

We hear here and there that Fable is awesome. I'm the first to say that because really it just saved me tons of hours of debugging and refactoring.

But yes, interop is not easy.

It's not strictly a Fable topic though: whatever the language, interop is not easy, has never been and will never be.

ts2Fable can help get started but also seem to block things too because it's never really precise to the point we can use it right away (ints being floats, obj when all else fails).

Especially when like PouchDB you have code splitted into several .ts files.

Like @MangelMaxime wrote, recursive modules have made things easier in most cases. Before that it was just crazy to find the proper order. But that's not enough. That will never be.

So we need to do some refactoring and then it appears as something painful because of a common psychological bias: we thought it would do all the hard work for us!

So I see things like this: ts2fable allows one to discover an API then starts the real editing work. ts2fable is very important and very powerful in this process. It really helps. But that should be its only goal: help us get started.

So I think either we find a way to interop seamlessly with js/typescript or we accept the compensation of writing bindings like artists ;)

Now, what we can do is share more if not support everything. I mean, I do have many bindings like everybody here but I don't want to share them that much because they're fitted to my purposes and do not map 100% of the libraries features and I won't really support them, add/deprecate features unless I need them. And it's usually used for 5 lines of code in a project.

But maybe we could start an interop network of gists for instance, and directly ping one another if we're stuck or need more information. Maybe before we add a proper section in the next version of Fable web site we could start by linking things in awesome-fable?

Meanwhile let's continue/start to share our interop experiences. What do you think?

@whitetigle
Copy link
Contributor

pinging @Zaid-Ajaj would help I think ;)

@drk-mtr
Copy link

drk-mtr commented Feb 20, 2019

@MangelMaxime Yes I think anything that anyone can do to share experiences would help, I find these posts really informative.

Considering the level of contributions the people on this thread have made, I find it hard to comprehend and quite inspiring that you manage to find the time in your day to be honest. Immensely thankful for the software and any documentation is a bonus. So I feel the onus is on people like me to document their experiences while learning (when I get to the point where I'll be assisting more than misleading!).

I've seen Zaid's guide, it's becoming my bible hehe.

To be fair, my attempts have generally been with monolithic libraries of the babylon / three variety, not little libraries and therefore a big ask.

One thing I'm curious about - when Ionide struggles with the large recursive file - I'm assuming there would be no way to tell it to do less checking, since that would mean it can't extract types for use elsewhere? Not sure whether it's worth asking Krzysztof as I don't know whether it's Ionide or on the compiler side and therefore out of his hands...

@MangelMaxime
Copy link
Member

This is true that Ionide is having a hard time when you have a large recursive file or if you got a lot of files in your project.

For example, when working on Fulma Ionide works fine 5-10min and after that, it can't keep up. However, when you have the bindings ready/working. If you create a nuget package then Ionide will use the .dll files instead of source file to get intellisense and it's working much better.

I think a way to check if this is a bug/limitation of Ionide you can test working with VisualStudio if your are on Windows and see if the problem stays.

@drk-mtr
Copy link

drk-mtr commented Feb 21, 2019

I'll give that a go - thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants