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

Add typescript support and update package.json #5750

Closed
wants to merge 11 commits into from

Conversation

SamTV12345
Copy link
Member

This pull request will add typescript support. At same time I had to update log4js so this is definetely v2 or v3 - so to say a major version. I'll try to complete this task as soon as possible but it requires renaming and fixing every file. Typescript does not like the old require() syntax.

But I could already spot a lot of typos in the files or unsafe access like accessing an undeclared field in javascript.

A big highlight is in node/handler/PadMessageHandler.ts where a Googler left a comment on the initialChangesets field.

@SamTV12345 SamTV12345 changed the title Added typescript support for most backend files. Add typescript support and update package.json Jun 22, 2023
@JohnMcLear
Copy link
Member

I imagine log4js change breaks lots due to change in their blob format.

@SamTV12345
Copy link
Member Author

Yes it will probably. But I don't see a solution other than adapting to that new version. There is no typescript support for the old version and with the vulnerabilities I don't know if it is good to stay very long on the old log4js version.

@webzwo0i
Copy link
Member

Do you think there is a way to get benefits from tsc while still keep using JS? I fear that this destroys compatibility with all of the plugins. There seem to be solutions for using JSDoc annotations (microsoft/TypeScript#33136). I think something like generating declarations, running tsc with allowJS could work. Adapt JSDoc, generate declarations and formalize types for our most important objects (as you already did in this branch) and run tsc adding one file at a time in Github's workflows - do you think such an approach could work? We won't get pure typescript, but we'll get a lot of insight re problematic code.

I experimented a lot with Facebook's flow and tsc in the past and you'll definitely spot a lot of long-standing issues that should be fixed. In fact I used it to track down issues (e.g. #5253 )
I tried approaches like pre/post-processing to generate JS files, which did not work quite well (e.g. because plugins require JS files directly). One other approach could be using a new state-of-the-art bundler, as this would introduce a pre-processing step anyway.

If you think the above will never work, is there kind of a roadmap for switching to TS? I assume plugins will work when releasing a 2.x version that is 100% TS, as all plugins are supposed to depend on 1.x. Afterwards, we start rewriting ether's plugins to depend on 2.x. In that case it would be worthwhile if 2.x would be stable asap, because due to the low developer resources we cannot support two versions.

Is TS capable of emitting JS that will likely work with our current system? If yes, this could be another approach, e.g. start a fresh project with TS that does not support most of the plugins yet, but we can use the new TS-based project to emit JS files for Etherpad.

Regarding log4js: I think it can be updated as https://github.com/ether/etherpad-lite/pull/5178/files is released for quite a while now. Or is there another problem I'm unaware of? There are not so many plugins that depend on log4js (maybe around 20).

I can hardly wait for all this to happen :-)

@SamTV12345
Copy link
Member Author

It is hard. I tested compiling js too but I get a lot of typescript errors when importing a js file. I think we should just transfer every file to typescript so we can start fresh and have no technical dept left. I ported the server part to typescript. Unfortunately with jQuery it is a bit too tricky. Do you know and understand still jQuery @JohnMcLear @webzwo0i ? If yes it would be awesome if you could port the frontend to esm and typescript.

I guess we are still a far cry away from a working etherpad in Typescript. We could deprecate the old JavaScript master branch and continue development on main with Typescript. We would also need to port the plugins maybe. I don't know how much work they are individually. If we split the branches we could first release a blank etherpad that only contains core and no plugins and then move on from there and port new plugins one by one. Those that use many plugins can then still remain on the old master branch.

It at least started half through. Maybe it will start again once everything is ported. The server depends on the frontend.

I think we need to move on at some point. Otherwise we will lack behind and have even more problems having a working version. 20 sounds viable.

Yeah. I am really exited. Something like Rollup would be possible and we could use a more modern frontend technology like React.

@JohnMcLear
Copy link
Member

@SamTV12345 sorry man but there is no way I can find time or the motivation to port code right now!

@SamTV12345
Copy link
Member Author

Thanks for the update. I'll try to continue this weekend. It will probably be one of the last weekends I have time for that. Other than that I can work full time on the project starting from 9 August. Maybe with enough time I can get it started.

@SamTV12345
Copy link
Member Author

I'm getting as far as it starts checking for plugins but then it doesn't continue and just exists.

@github-actions github-actions bot added the Stale No recent activity label Sep 3, 2023
@github-actions github-actions bot removed the Stale No recent activity label Dec 17, 2023
@AugustinMauroy
Copy link

Hey! Nice to see theses change. But why use babel instead of SWC?

@SamTV12345
Copy link
Member Author

Hey! Nice to see theses change. But why use babel instead of SWC?

Hey. Sorry didn't find much time for this pull request. Where do I use Babel?

@@ -0,0 +1,3 @@
{
"presets": ["@babel/preset-env"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SamTV12345 this file 🤗

Copy link
Member Author

@SamTV12345 SamTV12345 Jan 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hint. Good question. Now with SWC this shouldn't be needed anymore.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now with SWC this shouldn't be needed anymore.

normally yesss

@SamTV12345
Copy link
Member Author

@AugustinMauroy I opened a new pull request that looks a lot more promising than this one. I use ts-node to transpile the typescript files. If you'd like to you can help me with this one. I guess getting the types right will be easy, later the deployment stage where we transform typescript to javascript will be the hard part.

#6112

@SamTV12345 SamTV12345 closed this Feb 17, 2024
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

Successfully merging this pull request may close these issues.

4 participants