morethanwords / tweb

Telegram Web K, GPL v3
https://web.telegram.org/k/
GNU General Public License v3.0
1.77k stars 574 forks source link

[Feature Request] code refactor #19

Closed olegKusov closed 2 years ago

olegKusov commented 3 years ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

I'm little bit confused with codebase of telegram k. I Tried to learn it for about 3-4 days and it's really hard. All app like a big heap. Maybe i'm not so good in development, but it's really hard to understand this code. Usage of vanilla js with custom event listeners (rootScope), custom state managment, storages, a lot of promises, async logic and workers. And it's all in very long classes (1k-5k lines of code only for one class). It's unreal to keep in mind all this logic.

I'm not sure about simplicity of code maintaining. Maybe for Telegram contest it's okay but in the long term this raises questions. Most likely, new feature will lead more logic and more lines of code. And if you tried to not use React only because it take some KB of loading data, some future basic new features will inflate the codebase.

Describe the solution you'd like A clear and concise description of what you want to happen.

Global refactor of codebase

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

In ideal world - use just simple React. Vanilla is not the best choice for interfaces with big logic.

morethanwords commented 3 years ago

You're right, it looks like a big heap. However, it was refactored from legacy Webogram (almost every app...Manager was reused). I want to refactor it too, but it will take much time that I don't have now.

Answering about React: I mustn't use any UI framework (it was a rule of the contest) that will help me somehow in its developing, so I'm still writing it in pure JS and without any kind of virtual DOM. Of course it's hard to read, but you're getting no dependencies that will be discontinued in a few years. As a bonus, if you're able to read and work with it then you're real pro.

(I'm trying to do my best with its readability, though it's going so-so)

hanayashiki commented 3 years ago

when it is online and running, i think there's no need to stick to no UI framework?

but i think not using ui framework is very cool, keep your great work!

olegKusov commented 3 years ago

it was a rule of the contest

It’s really strange. People, who wrote this rule maybe don’t understand that lack of framework - bad idea for such big apps in 2021.

but you're getting no dependencies that will be discontinued in a few years

😂 react lives 10 years and there is no problem with it.

morethanwords commented 3 years ago

when it is online and running, i think there's no need to stick to no UI framework?

1/2 I'm not sure that it is still restricted to use framework. It hurts every time when I write document.createElement but it's pure & fast. Still I don't think that I need/have to rewrite it with any framework.

morethanwords commented 3 years ago

it was a rule of the contest

It’s really strange. People, who wrote this rule maybe don’t understand that lack of framework - bad idea for such big apps in 2021.

2/2 In my opinion, this rule isn't against the frameworks. It's about finding out the developer's level. Anyone can easily take a short courses and within 3 months he'll become a "React developer", so you're able to see the results of it: how many good projects (by good projects I mean with Telegram's quality) do you know?

And about discontinuation, Web grows and changes just by months. React is the one that's still surviving because of its simplicity, though it just pretends to be simple. If you don't know how to write the same application (read it with 'quality' here) without using any framework, most likely that you won't be able to do this with any of them.

That is, creating Telegram is not about creating simple forms and buttons, it's a very (very) big project with deep optimizations. That's why it requires storages, states and any other stuff that you can find here.

olegKusov commented 3 years ago

how many good projects (by good projects I mean with Telegram's quality) do you know?

Twitter, Facebook, Tesla, Airbnb, Netfix, Reddit. All top websites use React. And they use it because it give developer chance to think more about business logic and not about "hm, how I can handle my render and listeners?".

I agree with you, that vanillaJS is hard. You need to have some knowledge about vanilla js architechture. It's much harder to build architecture from scratch then React or any other framework. And vanilla always faster! It's true, but if we get Telegram K and Telegram Z and open them at the same time - Telegram Z loads chats much faster. And where is profit? I don't see it. Sorry. You are great developer and you can handle vanillajs project but it's just my opinion.

Update: I really love what you do. And love your work. You spent a lot of time to build this app and it's really deserves respect. Thank you for this great job, really. I do not want to hate someones work. I just wanted to discuss it. I ask you not to be offended in advance. And thank you for your answers.

morethanwords commented 3 years ago

It's true, but if we get Telegram K and Telegram Z and open them at the same time - Telegram Z loads chats much faster.

Loads chats much faster - do you mean that it loads (request) or renders faster? If it is about render, so yes, my application can be a little bit slower here, not because of vanilla, but because I did something wrong, I have two issues that I haven't fixed yet: background JS blur (it slows the render, it can be reproduced if you open the chat and then instantly will open another one. Will be fixed soon), and wrapping the original message text. I can optimize both of them. But it's not because of framework, only mistakes.

Thanks for discussion. If you see more places where performance is worse than second application has, let me know (except media viewer, it will be fully rewritten).

MachineR8 commented 3 years ago

In ideal world - use just simple React. Vanilla is not the best choice for interfaces with big logic.

Telegram (120916)_210511134946

morethanwords commented 3 years ago

but if we get Telegram K and Telegram Z and open them at the same time - Telegram Z loads chats much faster.

Please try again, should be much better now (though can do it even faster than now, but later).

olegKusov commented 3 years ago

but if we get Telegram K and Telegram Z and open them at the same time - Telegram Z loads chats much faster.

Please try again, should be much better now (though can do it even faster than now, but later).

Telegram Z after press to icon show chats for half a second. Telegram K need 3 seconds. Do you have caching? And know after adding app to desktop it opens with safari interface

morethanwords commented 3 years ago

Telegram Z after press to icon show chats for half a second. Telegram K need 3 seconds. Do you have caching? And know after adding app to desktop it opens with safari interface

Do you mean the PWA icon? It's because I don't have caching in service worker. I'll implement it during next week, so it won't load files every hour (cache-control is set to 1h).

For Safari problem: Could you please record a video with the bug?