-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Conversation
I imagine log4js change breaks lots due to change in their blob format. |
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. |
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 ) 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 :-) |
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. |
@SamTV12345 sorry man but there is no way I can find time or the motivation to port code right now! |
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. |
I'm getting as far as it starts checking for plugins but then it doesn't continue and just exists. |
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"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SamTV12345 this file 🤗
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
@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. |
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.