Google Summer of Code'23 @Zulip - Work Report
This summer I worked on migrating Zulip's Web frontend codebase from JavaScript to TypeScript. TypeScript is just JavaScript added with extra syntax for having strict type checking. It is an awesome language migrating to it improved the quality, maintainability, and reliability of Zulip's frontend codebase. Apart from this I also worked on doing many refactorings, breaking import cycles in the codebase, and also fixed some bugs 🐛.
So my project was to change extensions of some javascript files from .js
to .ts
and add some types to those files, sounds simple and straightforward right? Well it turned out not so straight forward :\ , Zulip's frontend codebase is huge around 300 files and still growing! Now to figure out type definitions for some modules I had to dig in all other modules linked with it and sometimes I had to dig in server code as well for getting most precise types, not to mention all the code refactoring needed for some modules. Figuring out types for some core data entities like Message
, StreamSubscription
, User
are especially hard because these entities are used throughout the codebase in lot of functions.
Import cycles are also a major challenge for this project, Zulip's frontend codebase have some quite large import cycles between lot of files, now to migrate one javascript module in those large cycles you either have to break that module out of that large cluster or handle all the modules of that cycle simultaenously and hence I also worked on breaking some import cycles throughout the codebase.
Finally, as one of my GSoC peer said - Every javascript file felt like a puzzle when migrating it to typescript. 😄
Since Zulip development community is absolutely amazing, I was active even before GSoC and hence I was really comfortable with community before the community bonding period started. So I ended up using that time to work on my project in addition to continue bonding with community, so the work I mention below includes PRs merged during the community bonding period as well. Those PRs are marked with a *.
For the sake of simplicity let's break my work in 4 sections:-
- TypeScript migration PRs
- Breaking import cycles and other refactoring PRs
- Bug fixes and others
- Participation in community/Code Reviews
Quick Links -
- Commits during coding period.
- Pull requests during coding period.
- All commits to zulip/zulip.
- All pull requests to zulip/zulip.
For figuring out type definitions for many of the below mentioned modules I used the zulip's api documentation which is very extensive!
-
#25435 ts: Migrate feedback_widget to TypeScript.*
Size: M | Lines added: +32/-11 -
#25658 ts: Convert confirm_dialog module to TypeScript.*
Size: S | Lines added: +5/-4 -
#25660 ts: Convert deprecated_feature_notice module to TypeScript.*
Size: S | Lines added: +18/-7 -
#25699 ts: Convert emojisets module to TypeScript.*
Size: M | Lines added: +29/-7 -
#25760 ts: Convert compose_textarea.js to TypeScript.*
Size: S | Lines added: +20/-4 -
#25755 ts: Convert internal_url.js to TypeScript.
Size: M | Lines added: +29/-16 -
#25599 ts: Migrate fetch_status to TypeScript.
Size: L | Lines added: +76/-19 -
#25756 ts: Convert web/shared/typing_status.js to TypeScript.
Size: XL | Lines added: +87/-94 -
#24559 ts: Convert web/src/billing module to TypeScript.
This PR migrated all the modules inside the
web/src/billing
directory to typescript. I usedzod
to create schemas for the api responses for better type safety. Zulip uses apage_param
dictionary which is prepared by the django server to initialize most of its frontend modules at the initial page load, now thispage_param
was being shared between every modules -billing
,portico
and the main Zulip website. So I also split up thispage_param
and created a seperatepage_param
module forbillling
module so that it is decoupled from the main website. I also fixed unit tests for this newly created billingpage_params
module.Size: XL | +260/-138
-
#25874 ts: Migrates web/shared/poll_data to typescript.
This PR ended up breaking all
poll_widget
across the apps as a regression, the bug caused every poll widget to render 0 votes on every options :(, I fixed the regression in a follow-up PR #26293 though :)Size: XL | Lines added: +308/-236
-
#26202 ts: Migrate setup.js to TypeScript.
Size: M | Lines added: +18/-28 -
#25539 ts: Migrate people.js to TypeScript
people.js
is one of the important core data module of entire Zulip frontend codebase, many other smaller modules are dependent onpeople.js
so it was very important to migrate this module to typescript. This PR actually follows the work done by my mentor @Zixuan Li in #24653, he already did lot of refactorings and prepared types for theUser
objects.So I only had to figure out the types for the
Message
objects which was actually a little bit challenging since we add some fields when processing theRawMessage
recieved from the server so I had to find out those extra fields and where they are added to carefully craft the correct types forMessage
.Actually this PR was getting big so it needed a prep PR as well - #26042, this prep PR basically was to address all the incomplete type
User
objects which we were creating from server data, we had to address this for better type safety.Since this was quite a large project I fully expected some regressions and we did saw one, we actually started seeing some unexpected error logs in sentry due to this migration, I fixed the regression in this PR - #26401.
(Prep PR) Size: XL | Lines added: +98/-44
(Main PR) Size: XL | Lines added: +341/-174 -
#26216 ts: Migrate peer_data.js module to TypeScript.
Size: L | Lines added: +31/-27 -
#26215 ts: Migrate user_group_pill module to TypeScript.
Size: M | Lines added: +52/-20 -
#25326 ts: Migrate list_widget to TypeScript.
This was one of those modules which was not a core data module but it's migration was still tricky because of how it is structured and due to it's generic nature. This PR was probably the biggest PR I handled ever :) It needed quite some refactoring to make it fully generic module. I used of course generic types for this module. It also needed lot of care to avoid breaking existing
list_widget
configurations throughout the app as I did withpoll_widget
😅Size: XL | Lines added: +651/-599
-
#26223 ts: Migrate message_overlay_ui to TypeScript.
Size: L | Lines added: +51/-39 -
#26237 ts: Migrate realm_playground.js to TypeScript.
Size: L | Lines added: +56/-21 -
#26384 ts: Migrate pm_conversations module to typescript.
Size: M | Lines added: +23/-12 -
#26381 ts: Migrate stream_data.js module to TypeScript.
stream_data.js
is also a core data module which handles stream subsciptions data throughout the app. Fortunately the types for thisStream
objects were already created, they were not exactly correct though I had to refine the existing types but that was less work than to create them from scratch!Size: XL | Lines added: +197/-120
-
#26497 ts: Migrate hash_util, browser_history, spectators to TypeScript.
Size: L | Lines added: +53/-42 -
#26488 ts: Migrate presence.js module to typescript.
Size: L | Lines added: +50/-17
-
#25516 Break dependencies between message_scroll and fetch_status.*
This PR helped in the typescript migration of
fetch_status
module by removing the dependency onmessage_scroll
. I did this by introducing a new modulemessage_feed_loading.ts
and put all the code which depended onmessage_scroll
there.Size: XL | Lines added: +90/-61
-
#25657 refactor: Move maybe_get_stream_name from stream_data to sub_store.*
This PR moves
maybe_get_stream_name
function fromstream_data
tosub_store
as it didn't have any dependency onstream_data
and it also helps us to cut off dependency onstream_data
for some of the modules includinguser_topics
.Size: L | Lines added: +49/-46
-
#25921 Breaking simple import cycles
This PR finished the work done in this PR #25871, both these PRs combined reduced the number of import cycles from 85 to 70 measured by this tool - Cycle Analysis Tool.
Size: XL | Lines added: +443/-406
-
#26052 Break some import cycles.
This PR breaks some more import cycles and brings down the metrics
126, 69
to118, 66
measured by the Cycle Analysis Tool.Size: L | Lines added: +69/-27
-
#25437 compose: Make it possible to schedule messages with wildcard mention confirmation.*
This PR was about to fix a bug which prevented users to schedule messages when the message contained a mention which pinged more than a set threshold of number of users. Also, while fixing this bug I found another bug which I reported here.
Area: compose | Size: XL | Lines added: +144/-97
-
#25642 people: Add version parameter for medium sized avatar urls.*
Before this PR when a user changed his/her user avatar it didn't got updated in real time, to fix this we needed to append the version parameter when constructing the urls for medium-sized images so that the browser updates the image in real time. This bug was reported here.
Area: real-time sync, uploads | Size: S | Lines added: +11/-4
-
#25700 settings_playground: Fix sorting issues in code playgrounds table.*
This PR fixes the sorting bug which we had on the code playgrounds settings table. The sorting function which we were using were incorrect and hence the sorting was inncorrect for this table. I used generice sort functions functionality of our
list_widget
to fix this bug.Area: settings | Size: M | Lines added: +4/-25
-
#25889 docs: Fix data type for submessages field on message type.
Before this commit our docs mentioned
string[]
data type for submessages field on the message object. This commit changes the type toobject[]
and correctly mentions all fields of the submessage object.Area: docs | Size: M | Lines added: +24/-2
-
#25936 starred_messages_ui: Add initialize method for this module.
This bug was caused by this commit 7ad3225, due to this bug starred message counts were not shown until the user stars or unstars a message.
Area: left sidebar | Size: S | Lines added: +7/-2
-
#24175 docs: Add documentation for delete emoji endpoint.
This endpoint was previously marked as
intentionally_undocumented
but that was a mistake. Removedintentionally_undocumented
and added proper documentation with validpython_example
for this endpoint. -
#26144 settings_emoji: Fix file upload bug in upload emoji modal.
Before this PR if a user pressed enter to submit the add emoji form in the settings, the uploaded emoji was getting cleared and were never getting uploaded. Fixed this bug in this PR and now users can submit the form by hitting enter key.
Area: popovers, settings | Size: XS | Lines added: +3/-2
-
#26115 dialog_widget: Prevent default action when submitting the form.
There was a bug in
dialog_widget
that caused the whole page to reload when the user submits the form present indialog_widget
, fixed that bug in this PR by preventing the default html behavior when submitting the forms.Area: settings | Size: M | Lines added: +9/-29
-
#25925 tests: Improve automated tests for submessages.
Added an additional test case to
test_submessages.py
for testing the message object containing submessages meta data. Previous to this commit we were never validating the submessage schema in the message objects.Area: documentation, testing-coverage, widgets | Size: M | Lines added: +31/-1
All of the zulip development related conversations happens on the Zulip iteself!, more specifically at CZO, although I have actively participated in many topics/threads I will list some of them here -
- #GSoC > Lalit - checkins: This is where I posted my regular checkins.
- #frontend > people.js typescript migration
- #frontend > typescript migration
- #frontend > simple import cycles
- #feedback > starred messages count
- #feedback > avatar live update
List of all PRs I reviewed -
- #25999 ts: Convert signup.js to TypeScript
- #26417 ts: Migrate stream_create_subscribers_data.js and user_group_create_members_data.js to TypeScript.
- #26418 ts: Migrate settings_ui.js to TypeScript.
- #26505 ts: Migrate compose_fade_helper.js , compose_fade_users.js, and recent_topics_util.js to TypeScript.
- #25864 ts: Convert part of web/src/portico module to TypeScript
There are still lot of files left to migrate to TypeScript, this include the files involved in major import cycles like compose
/narrow
family of modules, so we will need to either stub some modules to migrate that cluster or break some more import cycles. Most of the remaining files though are fairly easy to migrate to typescript now since most of the major core data modules are already migrated to typescript, so migrating the remaining modules will just require us to use already prepared types from the other modules.
Overall my GSoC experience with Zulip was wonderful, I got to learn so many new things! I got to learn the rebase-based git workflow, I had rarely used rebase
git command before contributing to Zulip, but I am glad I got to learn about it because it really helps to keep your PRs neat and tidy! I also picked up some more advanced typescript features, for example I learned how to make types with mutually exclusive properties using |
operator and a lot more.
Apart from the technical skills, I also got to learn the importance of code reviews, regular checkins, communicating in the community and in general I learned how to work in a large community to develop a software that thousands of people uses!
Since I had such a great time contributing to Zulip, I definitely plan to keep contributing at Zulip as well as many other open source projects out there :)
Can I join the team to contribute to the project. I have 2+ of experience working with JS/TS projects.