search

roccomuso / node-ads

NodeJS Twincat ADS protocol implementation
58 stars 21 forks source link

Typescript, Testing, Linting #37

Open jhenson29 opened 4 years ago

jhenson29 commented 4 years ago

Typescript is mentioned in another issues here https://github.com/roccomuso/node-ads/issues/14#issuecomment-526147603 and it's suggested that it would take a complete rewrite.

Not necessarily. The non-typing syntax of TypeScript is JavaScript. I've changed many projects (mostly non-OSS) over from JS to TS and it's relatively painless (especially if they are smaller, e.g. <10kLOC). I find that the only things I end up changing, other than adding types, are things that should have changed anyway, but weren't apparent without the types.

I think there is a significant benefit to both provide typing for end users as well as maintaining types in the project for future maintenance.

TypeScript can also address older node compliance (also mentioned in other issues/PR) by have TS compile to an earlier version of JS for publishing on NPM while allowing the source code to be maintained with the latest syntax.

I'd also like to see the addition of unit testing and linting to the project.

Would any or all of these changes (typing, testing, linting) be entertained?

roccomuso commented 4 years ago

Feel free to open PR :)

Units tests would be appreciated too

jhenson29 commented 4 years ago

Okay. I thought I’d ask first as I’ve had some people that are simply not interested in TS at all. I’ll review more this weekend.

roccomuso commented 4 years ago

I mean unit-test would be a good start. Rewriting in TS from scratch can lead to errors or break stuff. I'd suggest first to have a simple unit test PR