sepa.js icon indicating copy to clipboard operation
sepa.js copied to clipboard

Should sepa.js be rewritten to Typescript?

Open fellmann opened this issue 1 year ago • 13 comments

Because I need this for my project, I started to rewrite this library using TypeScript, a different XML builder and modern tooling. @kewisch Are you open to include this as "v2" in this repository?

EDIT from @kewisch: If you are reading this and using sepa.js in your code, could you put a thumbs up/down on if you think we should convert this to typescript instead? This will help inform a path forward.

fellmann avatar Jul 09 '24 06:07 fellmann

I'm not a big typescript fan. If you want the best of both worlds then we could do what we did for https://github.com/kewisch/ical.js , use jsdoc to generate the types. Would you be open to adapting that approach?

I think for this library my only other requirement is reducing dependencies that are not devDependencies. Since money is involved I'd rather not spend too much time auditing dependencies. If needed we could pin them and update with a code review.

I'm all up for modern tooling though, I almost started to make some updates recently because it was annoying me that the tools were so old/basic.

kewisch avatar Jul 09 '24 12:07 kewisch

I looked into the approach that @kewisch suggested and managed to add some jsdoc (as a poc). However, generating the .d.ts files uses TypeScript via rollup in ical.js, which in turn requires es modules.

At that point, it would imho be less effort to ~rewrite~ migrate sepa.js to TypeScript (with the additional benefit of type safety) and then use tsdoc to generate docs than to update the code to es modules, jsdoc and type definition generation. The two custom jsdoc plugins also have their own overhead.

leMaik avatar Jul 22 '24 13:07 leMaik

Sorry for my delayed answer. My (opionated) approach is not to write any new code in plain JavaScript. Compared to TypeScript, I see a lot of downsides and almost no advantage. Plus, when working with the code, I felt the approach a bit complicated and hard to extend, while the design of instantiating classes and setting properties subsequently does not play well with type-safety. (But, the concept was absolutely fine a few when this library was written.) In the meantime, I have started a rewrite using TypeScript and fast-xml-parser. It supports:

  • Direct Debit export
  • Transfer export
  • Statement import (partially)
  • Feature flags for different file versions
  • Modern build tooling Still missing a few test cases being rewritten, readme, examples etc.

Feel free to have a look at the current state and comment: https://github.com/fellmann/sepa.js/tree/rewrite

fellmann avatar Jul 22 '24 15:07 fellmann

I agree with you that it all comes down to opinion and preference, there is probably no right answer. There are many advantages to type safety in general. What I don't like about Typescript is the need for a transpiling/compiling step. I'd prefer to stay as close to the engine as possible, which makes debugging easier, avoids the runtime overhead, and keeps us more platform agnostic. It appears that the jsdoc approach plays well with IDEs doing type checking, so we'd get the best of both worlds.

The two custom jsdoc modules were a bit of a hack to get the docs to look like I expected (I wanted it to say e.g. SEPA.PaymentInfo instead of just PaymentInfo, while making it tsc compliant required just the class name). It doesn't appear there are any issues with type safety when using a toplevel object like SEPA, but I might just not have discovered it yet.

If it helps, I'd be willing to skip using a toplevel SEPA object, which would alleviate the need for those custom jsdoc plugins. Would you be willing to make compromises to maintain a new version using js + jsdoc? I'm happy to help migrate some of the tooling.

kewisch avatar Jul 23 '24 09:07 kewisch

I started migrating to js + jsdoc as a proof of concept. Maybe we should switch to es modules (similar to ical.js) while we're at it? I'm happy to help with the migration, but I'd feel uncomfortable with the idea of maintaining a (typescript or whatever) fork. It's nice to have people collaborating here to conform to the latest standards. :muscle:

leMaik avatar Jul 24 '24 12:07 leMaik

I agree, I'd love to continue working with you on this. I'm +1 on ES modules, there is sufficient support in browsers and node for this to be standard.

kewisch avatar Jul 24 '24 13:07 kewisch

Let me add some more arguments for my point of view:

  • I believe this library is almosts exclusively used in the backend as of today.
  • Even if it is used in the frontend, a bundler like webpack is used, and browser compatibility is ensured by transpiling the code.
  • In the backend, there is no native XML library available, so an external library like xmldom is always needed. Thus, it is better to use a modern and easy to use library instead of the cumbersome and old XMLSerializer.
  • The risk of introducing bugs within the library due to the complicated XML generation is much higher than bugs from using an external XML library.
  • The highest risk for bugs results from using plain JavaScript instead of a type-safe language.

fellmann avatar Jul 29 '24 06:07 fellmann

I'm seeing more and more people mention typescript in this repo. I think it all comes down to who is willing to stay with me on this journey. If those most likely to contribute patches feel more comfortable with typescript, then let's take that path. If this is more temporary and I'll be doing most of the maintaining, then I'd rather not make the jump to TS.

@leMaik How is your proof of concept going?

If folks reading this issue and making use of sepa.js could simply put a thumbs up or down on the initial issue, it might help give a measure on where the crowd stands.

kewisch avatar Sep 11 '24 10:09 kewisch

@kewisch As noted before, refactoring the code and adding jsdoc would probably be the same effort as rewriting it with TypeScript. Now that type declarations have been merged, i see even less reasons to add jsdoc.

leMaik avatar Sep 27 '24 09:09 leMaik

Happy new year! I'd be ok trying out the typescript approach for this library. New year, ready to try some new things :)

kewisch avatar Jan 05 '25 21:01 kewisch

Hi! I would be very interested in the typescript version of this library. Are there any news on this? @fellmann how is your rewrite going? Thanks!

dreinon avatar Feb 20 '25 15:02 dreinon

@kewisch I just created a PR fixing the package.json so the package can correctly be built to be used with typescript

dreinon avatar Feb 20 '25 15:02 dreinon

Hi! I would be very interested in the typescript version of this library. Are there any news on this? @fellmann how is your rewrite going? Thanks!

I currently don't have the time to work on this and don't need it in my projects. But I think the code in https://github.com/fellmann/sepa.js/tree/rewrite is working, feel free to use or create a library with it!

fellmann avatar Feb 20 '25 17:02 fellmann