8000 [WIP] Faster & Better OldTwitter by Ristellise · Pull Request #1055 · dimdenGD/OldTwitter · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[WIP] Faster & Better OldTwitter #1055

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

Merged
merged 31 commits into from
Jul 4, 2025

Conversation

Ristellise
Copy link
Contributor
@Ristellise Ristellise commented Jun 8, 2025

This PR is pretty Big (& Experimental!)

As such, I'm opening it early as a draft PR for everyone to see what I've been up to.

In general, this PR's idea is to remove the massive and hard to maintain html string block in favour of building the structure using JavaScript directly. (named "templating" in the below section)

I'm currently not allowing edits by maintainers. Once it's done I'll open up contributions.

This has 2 benefits:

  1. Reduced load times. (Using 6x CPU slowdown, rendering times has reduced from ~3s to ~0.7s from a previous experiment I did.)
  2. Reduced HTML sanitization needed across the entire html element is needed.

Additionally, I've rolled in a couple new changes:

  1. Updated twemoji to support unicode 16 emojis. (There's a new repo now maintained by the same ex-twitter staff that did twemojis)
  2. Updated purify.min.js
  3. Added JSdoc and comments where possible or I'm unsure.
  4. Apply prettify across the entire project (I think this is quite controversial but it is helping me in formatting headaches so...)
  5. Bumped the minor version to 1.9. I believe such a big change would need a minor version bump.

Potential concerns:

  • This hasn't been tested on older browsers (Specifically Internet Explorer, the rest of the browsers appears to have wide support over required JS features. A shim was needed but I believe it shouldn't affect much.)
  • Formatting might change. But I'll be fixing all the minor details.

What's done / not done:

  • constructQuotedTweet is now mostly templated. While there is some changes in terms of cleanup, visually it should be the same as before
  • [!] A lot of the html string based functions still emits strings for compatibility reasons
  • Currently working on the main tweet conversion to templating
  • Licensing? Probably? I used some StackOverflow code so...
  • The reply formatting code feels quite finicky. I believe there shouldn't be an issue from my testing but I'm can't find a tweet with the right attributes.
  • I havent wrote any changelogs yet. Someone could do it though.

What's after this?

  • Unsure. I probably don't want to maintain it forever lol but I'll be around for visual bugfixes (i.e. issue like: "It looked like this before and now it's different!")

Various Odds & Ends & Notes:

  • Before anyone asks: No, this isn't a LLM generated PR. This is an actual human that has been painstakingly converting a stringified html to a js templated version.

Ristellise added 18 commits May 12, 2025 22:26
- Console log can slow down updating of timelines when scrolling home page
- MediaTweets: For added parity (i.e. For my own userscript that listens to webrequests and captures data there)
 - Media elements are done
- fix elNew on prop element shenanigans
- More work on templated tweetConstructor
- Updated twemoji to support unicode 16.x
- TinyDomBuilder now supports class lists.
- Remove old template string Quoted functions.
- Started converting constructTweet functions
- Added missing `lang` to tweetviewer.js
 - Moved more code to js safe templates
 - Move dombuilder code to tdeb.js
@Impeta
Copy link
Impeta commented Jun 9, 2025

Wow, this is great! Thank you!
Since this is looking to be majorly reducing load times, do you think you could please take in mind about these commits 455cf77 0063446 82c6820 and just revert them in your own PR? So we can have visible link colors on timeline back and instant loading thanks to you.

@Ristellise
Copy link
Contributor Author
Ristellise commented Jun 9, 2025

Wow, this is great! Thank you! Since this is looking to be majorly reducing load times, do you think you could please take in mind about these commits 455cf77 0063446 82c6820 and just revert them in your own PR? So we can have visible link colors on timeline back and instant loading thanks to you.

Thanks for the interest! I'll have to look into the colors issue and see whether is feasible. It appears to be but I don't want to get people's hopes up too high.

Just an edit / update: I've finished the full conversion to templating for the html. I haven't pushed the code for it just yet but it should land real soon.

@Impeta
Copy link
Impeta commented Jun 9, 2025

Thanks for the interest! I'll have to look into the colors issue and see whether is feasible. It appears to be but I don't want to get people's hopes up too high.

Just an edit / update: I've finished the full conversion to templating for the html. I haven't pushed the code for it just yet but it should land real soon.

Even better, appreciate it! At the very least, if you aren't able to fix it yourself or make it better, you can just revert said commits and leave it as is, and write a parenthesis on the link colors options description (pictured here), warning users about it potentially slowing OT overall on loading timelines if enabled.
image

@Ristellise
Copy link
Contributor Author

Thanks for the interest! I'll have to look into the colors issue and see whether is feasible. It appears to be but I don't want to get people's hopes up too high.
Just an edit / update: I've finished the full conversion to templating for the html. I haven't pushed the code for it just yet but it should land real soon.

Even better, appreciate it! At the very least, if you aren't able to fix it yourself or make it better, you can just revert said commits and leave it as is, and write a parenthesis on the link colors options description (pictured here), warning users about it potentially slowing OT overall on loading timelines if enabled.

I just checked. seems like my PR doesn't contain those commits at all. I'll pull in those changes and uncomment those.

@dimdenGD
Copy link
Owner
dimdenGD commented Jun 9, 2025

If you want to return this option you'll have to rename its key to something different so that its reset for everyone and to make it off by default (and make it say that it can make oldtwitter like 5 times slower on some devices)

@Ristellise
Copy link
Contributor Author

If you want to return this option you'll have to rename its key to something different so that its reset for everyone and to make it off by default (and make it say that it can make oldtwitter like 5 times slower on some devices)

Alrighty, sounds good to me.

- On request from dimden. Default value for link colours is disabled.
Reading into a bit of time & date formatting, it seems that it can get a bit slow when calling toLocaleString.
If the browser is old enough, Intl is not available. As such, we fallback to using toLocaleString.
@Ristellise Ristellise marked this pull request as ready for review June 9, 2025 15:46
@Ristellise
Copy link
Contributor Author
Ristellise commented Jun 9, 2025

Opening it up for contributions now. I don't want to expand scope too much beyond the main slowdown oldtwitter. It's plenty fast now when zooming timelines (except I think the popup tweet viewer, that seems to be seperate)

There's other things that could be further optimized:

  • There's still a call to build the main html elements using raw tags. Ideally this could be converted but I think that might encroach on this PR a bit too much
  • Probably others if I remember correctly.

Some final timings (without CPU slowdowns, identical profile scroll):

Before:

renderTimeline 0.27229999999701976s
renderTimeline 0.22879999999701978s
renderTimeline 0.22059999999403954s
renderTimeline 0.2806000000089407s
renderTimeline 0.20079999999701978s

After:

[renderTimeline@profile] Took 0.053s to complete
[renderTimeline@profile] Took 0.046s to complete
[renderTimeline@profile] Took 0.045s to complete
[renderTimeline@profile] Took 0.043s to complete

@Impeta
Copy link
Impeta commented Jun 10, 2025

Testing through your own OldTwitter build and yeah, everything's working so far, no issues. I can tell the sheer reduced loading speed, nice.
Just one thing; if you have tackled on emoji coding inside, you might like to take a look on its emoji picker, that's been non-functioning for a while. More info: #820 (comment)

I think these got caught up in my experiment. It "works" but I don't exactly like how it's handled
@Ristellise
Copy link
Contributor Author
Ristellise commented Jun 10, 2025

Testing through your own OldTwitter build and yeah, everything's working so far, no issues. I can tell the sheer reduced loading speed, nice. Just one thing; if you have tackled on emoji coding inside, you might like to take a look on its emoji picker, that's been non-functioning for a while. More info: #820 (comment)

Did some digging. This warrants further look into tbh. Seems like when extending HTMLElement and calling an class(?) function when constructing the element doesn't seem to be supported in firefox. No idea why is the case. Doing some inital digging doesn't yield anything useful for me 😕

Doing a bit more digging, I came across this article. I'll try to implement the fix later when I'm not busy procrastinating away work 🤣

@Impeta
Copy link
Impeta commented Jun 16, 2025

@Ristellise Seems that for tweets with more than one uploaded video, it won't do anything if you click on them, play and so. Happens only on (your own) OT build.
Tweet for reference: https://twitter.com/zhenzhiwang/status/1933002826864578872

@Ristellise
Copy link
Contributor Author
Ristellise commented Jun 16, 2025

@Ristellise Seems that for tweets with more than one uploaded video, it won't do anything if you click on them, play and so. Happens only on (your own) OT build. Tweet for reference: https://twitter.com/zhenzhiwang/status/1933002826864578872

Thanks for bug report. I'll look into fixing it. Fixed. was a pretty simple mistake 🤣 (i.e. forgetting to set controls.)

Other than this, @dimdenGD, what else is needed for this to be merged into main?

@dimdenGD
Copy link
Owner

image
Seems like if you enable both views and bookmarks on posts, for some reason views appear as a second bookmark button

@Ristellise
Copy link
Contributor Author
Ristellise commented Jun 25, 2025

image Seems like if you enable both views and bookmarks on posts, for some reason views appear as a second bookmark button

That's... Interesting. I'll give it a lookie.

@Ristellise
Copy link
Contributor Author

Fixed. Seems like a simple mistake of copying and pasting bookmarks classList. Thanks for noticing it!

9E88

@dimdenGD
Copy link
Owner

Liking main tweets seems to be broken on separate tweet pages (for example https://x.com/notexttospeech/status/1937604725601673467)

@dimdenGD
Copy link
Owner

Also code should be formatted back to 4 spaces

@Ristellise
Copy link
Contributor Author

Reformatted to 4 spaces & fixed the missing element. Thanks for the catch.

@dimdenGD
Copy link
Owner

Well and last one thing is to fix what's failing the locales test

@dimdenGD
Copy link
Owner

I'm not sure what causes this, but some posts randomly get marked as if I bookmarked them, even though I didn't

@Ristellise
Copy link
Contributor Author

I'm not sure what causes this, but some posts randomly get marked as if I bookmarked them, even though I didn't

Super weird, did it happen previously?

@Ristellise
Copy link
Contributor Author

I'm not sure what causes this, but some posts randomly get marked as if I bookmarked them, even though I didn't

Oh I see it now.

image

@Ristellise
Copy link
Contributor Author
Ristellise commented Jun 30, 2025

Reminder to self to create PR fix. This returns an error, even though the tweet seems to be valid.

https://x.com/burny_tech/status/1845504728353939618

@dimdenGD dimdenGD merged commit 57189b4 into dimdenGD:master Jul 4, 2025
2 checks passed
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.

3 participants
0