Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to TypeScript #76

Open
bkiac opened this issue May 22, 2022 · 6 comments
Open

Migrate to TypeScript #76

bkiac opened this issue May 22, 2022 · 6 comments

Comments

@bkiac
Copy link
Contributor

bkiac commented May 22, 2022

I think it would be great to migrate the source code to TypeScript:

  • type tests can be removed because the compiler would catch the errors
  • JSDocs in a single file (the reason I'm copying the documentation from the JS files now is that it shows up in the context window on hover in IDEs)
    image
  • I find that TypeScript is generally easier to maintain; it would be easier for other developers to contribute to the project

The biggest blocker is getting the build, test, and publish process right. Then we can refactor incrementally.

We can keep the TypeScript rules lenient initially, so the development process doesn't have to change.

@alexbosworth, what do you think? I can set up an initial PR with the build tools, and after that, it should be straightforward to refactor individual files.

@alexbosworth
Copy link
Owner

I don't write in TS though so it's harder to maintain and write for me

@alexbosworth
Copy link
Owner

I think a way forward here is to further abstract out the methods in lightning and migrate also to the ESM model

A problem I want to address is that in order to import this library you need to interpret all the code, when maybe you only need to make 1 call. That can make running this pretty slow for the type of application that isn't long-running. So the abstraction here would be to make new modules for individualized methods so that your import would only compile the minimal set of code that you are interested in using.

The lightning library could then import the sub-library itself and maintain compatibility and also unified testing, while individual per-method sub libraries would actually have the code

@alexbosworth
Copy link
Owner

I tried migrating to ESM in the past but it was a huge undertaking and I gave up, so a more piecemeal approach could be helpful to iteratively approach the conversion. Also I'd like to target 100% unit test coverage more aggressively, so that could be an additional goal for abstracted out modules

@alexbosworth
Copy link
Owner

Individualized method libraries could be written in typescript or node.js, from the perspective of the lightning library it wouldn't matter and they could be seamlessly switched between, so long as they followed the same method signature

@bkiac
Copy link
Contributor Author

bkiac commented May 23, 2022

I don't write in TS, though, so it's harder to maintain and write for me

Yeah, I get that; that's why I thought a lenient TypeScript config would be better that allows JavaScript files also; the only difference between the current setup and this setup is that the types would be defined in the same module, and the extension would be .ts instead of .js

Sometimes, there are quirks with TypeScript, so I understand if you feel like it's not worth it.

I'm not too familiar with ESM, but esbuild can output a bundle in ESM format, and I think no code changes would be necessary; it might be worth a try if you haven't tried it before: https://esbuild.github.io/api/#format-esm. I can open a PR for this if you want.

@alexbosworth
Copy link
Owner

Sure I can take a look at that if I get around to moving methods out of the repo and take another attempt at ESM migration. Then the methods out of the repo would be imported into lightning repo just like the lightning methods were imported into the ln-service repo and migrated from there to here over time

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

No branches or pull requests

2 participants