Unify JavaScript Files to TypeScript, Update tsconfig.json, and Add VS Code Debugger Support

danfercf1 commented 3 months ago

Summary

I would like to propose a feature that unifies all JavaScript (.js) files in the project into TypeScript (.ts). This proposal also includes updating the tsconfig.json configuration to output build files into a dedicated folder and adding support for the Visual Studio Code (VS Code) debugger. These changes will enhance code quality, project organization, and developer experience.

Motivation

Type Safety: Migrating to TypeScript will improve code reliability by adding type safety, helping to catch errors during development rather than at runtime.
Consistency: Having the entire codebase in TypeScript will make it more maintainable and easier for new contributors to understand and contribute.
Organized Build Output: By configuring the tsconfig.json file to compile TypeScript files into a dedicated output directory (e.g., ./dist), we can keep our project structure clean and organized.
Enhanced Developer Experience: Integrating VS Code debugger support will make it easier to debug TypeScript code directly within the IDE, improving the overall development workflow.

Suggested changes

Convert All .js Files to .ts: Identify all existing JavaScript files and migrate them to TypeScript. This includes adding appropriate type annotations and ensuring each file is fully functional. Test the converted files to ensure there are no regressions.
Update tsconfig.json:
- Modify the tsconfig.json file to include the following:
  - OutDir: Set outDir to output compiled files into a dedicated folder such as ./dist.
  - RootDir: Set rootDir to the src directory to maintain a clear separation between source and output files.
Add VS Code Debugger Support:
- Create or update the .vscode/launch.json configuration file to support debugging TypeScript files within VS Code.
Update Documentation:
- Update the project’s documentation to reflect these changes, including any new instructions for running the build process, debugging, or contributing with TypeScript.

Implementation Details

Phased Migration: The conversion from JavaScript to TypeScript should be done incrementally to avoid disrupting the project. Each module can be converted individually and tested before moving on to the next.
Testing: Ensure that all tests pass after each conversion, and update or add tests as necessary for the TypeScript files.
Debugger Verification: After configuring the VS Code debugger, verify that breakpoints and step-through debugging work correctly with the compiled TypeScript code.

Potential Impact

Positive Impact:
- Improved code quality and safety due to TypeScript’s static typing.
- Cleaner project structure with separated build artifacts.
- More efficient debugging within VS Code.
Negative Impact:
- Initial time investment required for migrating files and updating configurations.

Next Steps

Discussion and Approval: I invite the maintainers and contributors to discuss this proposal and provide feedback.
Branch Creation: Once approved, a dedicated branch can be created for the migration to keep the changes isolated.
Implementation: Begin the migration of .js to .ts, update tsconfig.json, and add VS Code debugger support.
Testing and Documentation: Ensure everything works correctly and update documentation as needed.
Merge: After successful testing, merge the changes into the main branch.

grunch commented 3 months ago

Hi @danfercf1 this is a really well explained plan for migration to TS, I think we need to keep improving the project and make it more readable for new contributors and at the same time more sustainable over time.

@knocte could you read it ? what do you think about it?

knocte commented 3 months ago

@grunch hey,

I like the optimism at display here, and I don't disagree with any of the points.

But before I give my opinion on the JS->TS task, can @danfercf1 please explain why is this github issue mentioning 3 things at the same time? Why not separate them? I don't understand why they need to be together.

danfercf1 commented 3 months ago

@grunch hey,

I like the optimism at display here, and I don't disagree with any of the points.

But before I give my opinion on the JS->TS task, can @danfercf1 please explain why is this github issue mentioning 3 things at the same time? Why not separate them? I don't understand why they need to be together.

It's just to keep in mind the tasks to do, I think the addition of configurations for vscode debugger isn't related but it's a small change and shouldn't affects all the process, by the way it could be another task or feature to be added in order to have more atomic tasks

knocte commented 3 months ago

I think the addition of configurations for vscode debugger isn't related but it's a small change

If it's a small change, the more reason to split it into a different issue, which can be fixed by a specific PR that only fixes that!

it could be another task or feature to be added in order to have more atomic tasks

Yes let's keep tasks as small as possible, no need to munge together a bunch of things like if we were product managers. We're all developers here.

grunch commented 3 months ago

I agree with split it into smaller issues, I'm having head aches testing big PRs on this project 😢

grunch commented 3 months ago

Approved this request, and as we commented let's split this into several issues, as we are receiving donations we currently have budget to support the work of collaborators, for this one we have 500k sats for the whole work, @danfercf1 are you able to take this one? you have priority to be assigned as it was your idea, @knocte we have sats for you as a reviewer or developer too.

danfercf1 commented 2 months ago

Yes, I'm interested on it, I will split the tasks into other ones

knocte commented 2 months ago

@knocte we have sats for you as a reviewer or developer too.

Ah thanks, and will I be able to re-invest these sats to pay other contributors to fix other things?

let's split this into several issues

Yeah, I see 3 things so 3 issues?

Approved this request

BTW I'm surprised you "approved" this, given that 1 of those 3 issues would be "finish converting from JS to TS", and wouldn't this task in itself need to be split in many other issues? Especially given the fact that me, webwarrior and Mershad have been trying this for several months and we're finding that it's not that easy to convert without finding regressions (so the smaller the conversion the better).

grunch commented 2 months ago

@knocte we have sats for you as a reviewer or developer too.

Ah thanks, and will I be able to re-invest these sats to pay other contributors to fix other things?

Yes I wan to give away donations to contributors, we have this rewards board here

https://github.com/orgs/lnp2pBot/projects/2/

And also I want to give back to you, webwarrior and mershad for all the work you have done and keep doing.

Approved this request

BTW I'm surprised you "approved" this, given that 1 of those 3 issues would be "finish converting from JS to TS", and wouldn't this task in itself need to be split in many other issues? Especially given the fact that me, webwarrior and Mershad have been trying this for several months and we're finding that it's not that easy to convert without finding regressions (so the smaller the conversion the better).

Yes, there is no bad intention, I agree that this is a complex issue that is not possible to be done by only one dev, what you guys propose? how we can make this process work better? I'm thinking on split this into several issues and @webwarrior-ws , @Mersho , @danfercf1 and you can start take issues and keep working on your own pace, but if you have any other proposal we of course can discuss it 😃

danfercf1 commented 2 months ago

Yes, this kind of job needs to be splitted into more atomic tasks, but I think the must to have in any project is to have a really functional debugger configuration, I could start with that, and we can make a better planning in order to have a js to ts migration without regressions, I think we could have a time to talk in a meet about this in order to have all the stuff on mind

danfercf1 commented 1 month ago

Hi @grunch, what do you think if I close this issue and I will create another one to configure the tsconfig.json and for the debugger, I think it could be added the folder for the build as well

grunch commented 1 month ago

Hi @danfercf1 let's discuss it with our other contributors @knocte, @webwarrior-ws @Mersho, what do you think about how we should continue ?

danfercf1 commented 1 month ago

I realized that migration to TS has a good completed percentage and I think after it finishes, the main task to take in count is to define a build folder, it's because the ts to js compilation generates the compiled js files in many places and it creates a mess when you need to commit files. It could be part of the last task of the JS to TS migration. About the debugger it could be an isolated task or feature to add after the TS migration finishes it regards to define correctly the configuration just for a TS debugger

grunch commented 4 weeks ago

I realized that migration to TS has a good completed percentage and I think after it finishes, the main task to take in count is to define a build folder, it's because the ts to js compilation generates the compiled js files in many places and it creates a mess when you need to commit files. It could be part of the last task of the JS to TS migration. About the debugger it could be an isolated task or feature to add after the TS migration finishes it regards to define correctly the configuration just for a TS debugger

I agree @danfercf1 let's finish the JS to TS migration, then we work on the build folder

danfercf1 commented 3 weeks ago

I think this issue can be closed I will create just another one to add the vscode debugger

knocte commented 2 weeks ago

I think this issue can be closed

Yes let's close this issue, and create smaller ones.

WRT JS->TS conversion, actually we have it basically finished on our end, but wouldn't it make more sense to tag a new release first? before we propose next PR. @grunch which version is LNp2pBot-Production running on?

grunch commented 2 weeks ago

I think this issue can be closed

Yes let's close this issue, and create smaller ones.

WRT JS->TS conversion, actually we have it basically finished on our end, but wouldn't it make more sense to tag a new release first? before we propose next PR. @grunch which version is LNp2pBot-Production running on?

makes sense, the bot is running version 0.11.0, let's release a new version

lnp2pBot / bot