stil4m / elm-syntax

Elm syntax in Elm
MIT License
92 stars 26 forks source link

Get rid of encoding/decoding #197

Open jfmengels opened 10 months ago

jfmengels commented 10 months ago

This package supports parsing Elm code from a String to an AST, but it also supports encoding/decoding the AST to JSON.

In practice, I find that this encoding/decoding works is context-dependent. For instance, for elm-review, we switched to using a custom codec using (a vendored version of) elm-serialize which transforms into bytes (or something close to it).

This project has never had a guarantee that the same AST will be encoded/decoded correctly across versions. For instance, an AST encoded using v7.2.8 might not be decoded correctly using v7.2.9., which can be problematic and surprising to some consumer projects.

My proposal is to get rid of this feature of the project. Removing the JSON part of this package would remove the complexity of the package and remove some dependencies, it will also remove some modules that are in practice almost only there because of the JSON part (Elm.Syntax.Comments for instance). It will also let users decide how to handle this part when the version of the package changes, instead of hoping that it will be compatible.

I don't know if we should have a gist explaining how to do the JSON transformation which people can then adapt, or whether we should release this in a separate package still.

MartinSStewart commented 10 months ago

Yeah, I'm fine with removing json encoding/decoding. That said, I think we should take all that code and put it into some examples folder. I'm sure people will still need to do json encoding and having all the code available as an example that they can then tweak will save a lot of time.