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

[WIP] Separate relevant parts into their own "mixin" classes / continue adding missing pieces #25

Closed
wants to merge 18 commits into from

Conversation

watzon
Copy link
Contributor

@watzon watzon commented Apr 18, 2020

So this is going to be a big one. I just wanted to get it in as a PR so I can get feedback on it as I work. It's nowhere near finished yet, but it is at least testable in many cases. So far it entails two main pieces:

  1. Break TelegramClient apart into smaller, more manageable pieces. This is inspired by Telethon and handled in a very similar way (which I was surprised is actually very possible with JavaScript).
  2. Add missing pieces, allowing the library to be used in a much more friendly way. This includes all the convenience wrappers for handling updates, messages, users, etc.

I'm definitely open to comments. Yes, this is basically just a straight port of the way Telethon does things, no I am not creative enough to come up with my own "unique" way of handling things. Telethon just does things very well and I didn't see a reason to diverge too much. So props to Lonami 😄

@painor painor self-assigned this Apr 18, 2020
@@ -11,4 +11,5 @@ const { TelegramClient } = require('../gramjs')
await client.connect()

console.log(await client.getMe())
await client.disconnect().then(() => process.exit(0))
Copy link
Member

Choose a reason for hiding this comment

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

This will make it only work in node environments and not in browsers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair, I only had Node in mind for that example

@painor
Copy link
Member

painor commented Apr 18, 2020

Good PR. Although I am not familiar with Mixin so I'm going to assume it's just a hack to do multiple inheritances as python does.

I'm going to test it more and give some feedbacks.

@watzon
Copy link
Contributor Author

watzon commented Apr 18, 2020

Yeah it's basically a hack, although one that works pretty well. The standard way to do mixins is to just use Object.assign, but it has some drawbacks. Mainly that super doesn't work. This has a fairly good explanation of how this version of mixins works, and why it's better than the Object.assign method.

@watzon watzon marked this pull request as draft April 18, 2020 23:45
@watzon watzon mentioned this pull request Apr 20, 2020
@painor
Copy link
Member

painor commented Feb 9, 2021

I'm closing this in favor of #51

@painor painor closed this Feb 9, 2021
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.

None yet

3 participants