Commit Graph

141 Commits

Author SHA1 Message Date
Parvesh Monu 4bf8aaf2e0
Compress Images for Status Screen and Add Image Compression Script (#21300) 2024-09-23 16:44:24 +05:30
Icaro Motta 4b8a612df4
chore(docs): Document some of our existing testing practices (#20691)
Document some of our current testing practices in hopes of helping reduce
friction in PRs and communication in general. In theory, nothing in the text
should be a surprise because these are things we have been discussing over many
months (some things for almost 1.5 years) and are already present in the code.
2024-07-23 23:45:14 -03:00
Mohamed Javid a049fd341e
chore(docs)_: include new status-go tagging command in the merging PR process (#20607)
Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
2024-07-04 15:37:27 +05:30
yqrashawn ef9cc55501
feat: check before syncing doc (#20504) 2024-07-02 16:36:42 +08:00
Icaro Motta 615ad2f02b
dx(debug)_: Add FlowStorm, a tracing debugger for Clojure(Script) (#20054)
Adds FlowStorm https://github.com/flow-storm/flow-storm-debugger v3.7.5, a well
known (tracing) debugger for Clojure(Script).

With FlowStorm, you can debug almost any cljs function in status-mobile. And
although it is not as capable as on the JVM, its main features work well enough.

How do I use it? Please, check the markdown diff in this PR: doc/debugging.md.

When would you use FlowStorm in status-mobile? You can use it all the time if
you want, but FlowStorm can be a powerful tool to understand complex pieces of
code. Consider those large subscriptions or event handlers. Or all those
components with lots of bindings and calculations. Understanding some of these
things is no easy task, even with a REPL. It is not a replacement for re-frisk,
those are very different tools and each have their place.

Resources:

- Repository: https://github.com/flow-storm/flow-storm-debugger
- Documentation: https://flow-storm.github.io/flow-storm-debugger
- Features: https://github.com/flow-storm/flow-storm-debugger#features
- YouTube demos: https://github.com/flow-storm/flow-storm-debugger#some-demo-videos-newers-at-the-top
2024-06-03 19:47:10 -03:00
Andrea Maria Piana 711bcb2507 Add info on ci building pipeline for release 2024-05-27 13:23:51 +01:00
Churikova Tetiana ea4578b3e0 pipleine: update doc 2024-04-18 15:31:47 +01:00
Ibrahem Khalil f3aeddb53a
Update `doc/troubleshooting.md` (#19673) 2024-04-16 17:46:14 +02:00
Lungu Cristian aeef913f63
Fix chat permission-context (#19284)
* feat: moved permission-context logic to quo component

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* feat: multiple-token-gating and previews

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* fix: refactored quo component

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* feat: request join community on press

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* feat: added shadow (rn-shadow-2)

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* fix: formatting

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* test: added tests for permission-context

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* feat: added blur

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* fix: adjusted shadows

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* fix: added shadow mocks

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* fix: shadow rendered below the context

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* fix: addressed review comments

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>

* fix: replaced seq with string/blank?

---------

Signed-off-by: Cristian Lungu <lungucristian95@gmail.com>
2024-04-08 17:14:07 +03:00
Tetiana Churikova 354cda3be5
e2e: updating PR pipeline (#19478) 2024-04-03 18:00:04 +02:00
Siddarth Kumar da25280555
clean up unused libraries (#19456)
## Summary

This PR removes all of the unused `npm` libraries and their corresponding `gradle`/`cocoapods` dependencies.
The list of npm dependencies removed are :
- `@babel/preset-typescript`
- `create-react-class`
- `react-native-haptic-feedback`
- `react-native-image-viewing`
- `react-native-languages`
- `react-native-randombytes`
- `react-syntax-highlighter`
- `rn-emoji-keyboard`
- `tdigest`

This saves us some bytes in the bundle size.

## Platforms
- Android
- iOS
2024-04-02 16:32:54 +05:30
Jakub Sokołowski 9e71fc2d14
ci: make contract tests stage optional
Signed-off-by: Jakub Sokołowski <jakub@status.im>
2024-03-27 14:12:28 +01:00
flexsurfer b0133e97cf
UI components coding guidelines (#18926)
* UI components coding guidelines
2024-02-23 18:30:46 +01:00
Siddarth Kumar 3c12ac870a
chore: fix few outdated things in docs & readme (#18920) 2024-02-22 00:17:09 +05:30
Volodymyr Kozieiev ae75eaeb33
Link to Malli presentation added to docs (#18849) 2024-02-20 12:43:44 +00:00
Siddarth Kumar 42cab08553
chore: Improve DX for building the app locally (#18784)
After upgrading `react-native` to `0.72.5` we frequently started seeing the _red screen of death_ on both `Android` and `iOS` simulators right after the app was built and installed.
This used to happen because our workflow required us to do the following :
- `make run-clojure`
- `make run-metro`
- `make run-ios` OR `make run-android`

The problem with this approach was after `metro` was started the `iOS`, `Android` build step would change the files that `metro` couldn't handle and hence metro would go out of sync.
The quick fix back then was to restart `metro` terminal and to open the app again from the simulator.
This was however not a good DX.

This commit fixes that.
We no longer rely on `react-native` cli to generate and deploy debug builds on simulators. We take control of the process via our own script. The new workflow introduced in this commit will first build the app, then install the app on the simulators and then start metro terminal. When `metro` is successfully running the script will then open the app.

The new workflow now is :
- `make run-clojure`
- `make run-ios` OR `make run-android`
2024-02-14 19:58:45 +05:30
Lungu Cristian 901818492d
Replace unmaintained biometrics package (#18531)
* feat: added react-native-biometrics dependency

* chore: added malli schema to auth

* fix: malli schema

* feat: using react-native-biometrics

* fix: removed biometry not-enrolled error supression

* feat: added check for enabled biometric

* fix: biometrics error handling on ios

* chore: remove touch-id library

* chore: cleanup

* removed proj.list dep

* fix: gradle get_projects regex edge-case

Handles cases when the gradle project has a description, which shows up when running `gradle projects` as (`react-native-biometrics` - react-native-biometrics), breaking `make nix-update-gradle`. Here we're just adjusting the regex to ignore everything in the line after the closing (`).

* build: ran "make nix-update-gradle"

* chore: comment typo

* chore: replaced old lib in test mocks

* fix: addressed review comments

* fix: using event for standard-auth biometrics

* ref: using ex-cause for biometric error codes

* fix: removed promesa changes

* fix: auth slide biometric success not triggered
2024-01-29 13:33:46 +02:00
Ulises Manuel 5cc027a1f5
[#18461] Update guideline about apply-animations-to-style (#18462) 2024-01-11 19:17:07 -06:00
Icaro Motta b4640dabb9
Run integration tests separately from unit tests (#18304)
- make test still exists, so if you have been using it, as well as make
  test-watch, they should all work exactly the same.
- [Changed] As part of the check stage, Jenkins will run Lint and Unit Tests
  in parallel. Integration Tests run later because running them at the same
  time as Unit Tests caused errors.
- [Added] "make unit-test" and "make unit-test-watch" run unit tests only.
  Watching all unit tests is faster now because we ignore integration tests and
  we only compile shadow-cljs :mock target once. We are at approximately 10-15s
  to re-run all unit tests after saving a watched file, depending on your
  hardware. If you change mocks.js_dependencies.cljs, you must re-run the make
  target.
- [Added] "make integration-test" and "make integration-test-watch" run
  integration tests only. These are much slower than the unit tests.

Fixes https://github.com/status-im/status-mobile/issues/18112
2024-01-05 15:47:03 -03:00
Icaro Motta 563f1c588d
Improve test failure readability (#18049)
Problem: failed equality checks as in "(is (= expected actual))" will give a
single, long line of output that for anything but the simplest data structures
is unreadable by humans, and the output doesn't give a useful diff.

Solution: use library https://github.com/nubank/matcher-combinators and its test
directive "match?" which will pinpoint where two data structures differ. Then,
instead of "(is (= ...", use "(is (match? expected actual)". It works
beautifully.

The library offers other nice matchers, but the majority of the time match? is
sufficient.

Can we use another test runner like Kaocha? kaocha-cljs2
(https://github.com/lambdaisland/kaocha-cljs2) would be able to print better
test errors out of the box, among other features, but I have no clue if it would
work well or at all in our stack (in theory yes, but it's a larger piece of
work).
2023-12-05 17:20:54 -03:00
Volodymyr Kozieiev bfa23c182f
Add requirement for component tests (#18015) 2023-11-29 18:50:49 +00:00
Andrea Maria Piana 27e27aa113
Add contribution status-go doc 2023-11-09 15:34:20 +00:00
Alex Tumanov 569036c1d8
Add a button to address watch screen; (#17781)
Add account creation screen

remove icons

remove extra utility and create a new one that would use conventional way of getting an emoji

fix lint

Use button component instead of bottom-actions

Provide global customization color to buttons

Use conventional approach to extract account name

Move to another address

Move to another namespace

Refactor bottom-actions to have button props in maps

Update doc with new icon location

Add spaces between styles

Work on pr comments

Use :on-change-text instead of :on-change for input component
Subscribe to :profile/customization-color directly
Use bottom button from the create-or-edit-account wrapper

Remove extra code
Sort requires

Move ns to proper fileˆ

Fix styles
2023-11-07 16:40:24 +00:00
yqrashawn 701df811b0
feat: lint direct require quo component outside src/quo (#17828) 2023-11-07 10:21:58 +08:00
Dmitri Akatov b47c97a4fd
Mark utils.re-frame/defn as deprecated (#17788)
Also suggest to use utils.re-frame/reg-event-fx instead of utils.re-frame/defn
2023-11-06 19:09:40 -03:00
Siddarth Kumar 4ac7f0bdda
update docs & makefile for iPhone 13 (#17784)
The design team has now decided to keep iPhone 13 as the baseline standard instead of iPhone 11 Pro.

This commit updates the docs on pixel perfection and starting guide.
We also update the default simulator to iPhone 13 for `make run-ios`
2023-11-06 23:55:06 +05:30
Smoothieewastaken 55b233c528
docs: Fixed typos (#17749) 2023-10-30 08:06:52 +01:00
Lungu Cristian 954373105d
Visual debugging of components (#17729)
* feat: debug/show components from the REPL

* ref: some renames and cleanup

* chore: removed comments/imports

* feat: added remount on every eval & small changes

* doc: added tip to debugging doc

* doc: formatting

* doc: more formatting

* fix: fixed docs and other review comments

* fix: moved & renamed into the dev.component-preview ns

* style: adjusted debugging readme spacing

* fix: removed trailing line
2023-10-27 13:14:01 +03:00
Icaro Motta c0b21aa315
Document how to auto-format Clojure files in VSCode with zprint (#17719)
Document how a VSCode user can configure their editor to auto-format
Clojure(Script) files that respect all the formatter (zprint) settings specified
in status-mobile/.zprintrc.

The result should be that on every file save, the file should be identically
formatted as if you used make lint-fix, at least the part about zprint. If
lint-fix does other things now or in the future that are outside the scope of
zprint, those won't be covered by the VSCode extension.
2023-10-25 23:19:56 -03:00
Flavio Fraschetti 3f1d52da8c
Update README (#17674)
Add Status App Functionality Demo to docs
2023-10-18 14:33:05 +01:00
Icaro Motta 6e897f0ea6
Migrate away from rf/defn and rf/merge (first step) (#17451)
This commit shows how to move away from rf/defn
f12c7401d1/src/utils/re_frame.clj (L1-L90)
& rf/merge
f12c7401d1/src/utils/re_frame.cljs (L39-L85)
and why we should do it.

## Problems

Before jumping to solutions, let's understand the problems first, in no order of
importance.

### Problem 1: Cyclic dependencies

If you ever tried to move event handlers or the functions used inside them to
different files in status-mobile, you probably stumbled in cyclic dependency
errors.

When an event is registered in re-frame, it is globally available for any other
place to dispatch. The dispatch mechanism relies on globally unique keywords,
the so called event IDs, e.g. :chat/mute-successfully. This means that event
namespaces don't need to require other event namespaces, just like you don't
need to require subscription namespaces in views.

rf/merge increases the likelihood of cyclic dependencies because they force
event namespaces to require each other. Although not as common, this happened a
few times with devs in the team and it can be a big hassle to fix if you are
unlucky. It is a problem we should not have in the first place (at least not as
easily).

### Problem 2: We are not linting thousands of lines of code

The linter (clj-kondo) is incapable of understanding the rf/defn macro. In
theory, we could teach clj-kondo what the macro produces. I tried this, but gave
up after a few tries.

This is a big deal, clj-kondo can catch many issues and will continue to catch
more as it continue to evolve. It's hard to precisely count how many lines are
affected, but `find src/ -type f -name 'events.cljs' -exec wc -l {} +` gives us
more than 4k LOC.

### Problem 3: Blocking RN's UI thread for too long

Re-frame has a routing mechanism to manage events. When an event is dispatched,
it is enqueued and scheduled to run some time later (very soon). This process is
asynchronous and is optimized in such a way as to balance responsiveness vs the
time to empty the queue.

>[...] when processing events, one after the other, do ALL the currently queued
>events. Don't stop. Don't yield to the browser. Hog that CPU.
>
>[...] but if any new events are dispatched during this cycle of processing,
>don't do them immediately. Leave them queued.
>
>-- https://github.com/day8/re-frame/blob/master/src/re_frame/router.cljc#L8-L60

Decisions were made (way back in 2017) to reduce the number of registered
re-frame events and, more importantly, to coalesce events into bigger ones with
the rf/merge pattern. I tried to find evidence of real problems that were trying
to be solved, but my understanding is that decisions were largely based on
personal architectural preferences.

Fast-forward to 2023, and we are in a situation where we have many heavy events
that process a LOT of stuff in one go using rf/merge, thus blocking the UI
thread longer than we should. See, for example,
[status-im2.contexts.profile.login.events/login-existing-profile](3082605d1e/src/status_im2/contexts/profile/login/events.cljs (L69)),
[status-im2.contexts.profile.login.events/get-chats-callback](3082605d1e/src/status_im2/contexts/profile/login/events.cljs (L98)),
and many others.

The following excerpt was generally used to justify the idea that coalescing
events would make the app perform better.

> We will reduce the the amount of subscription re-computations, as for each
> distinct action, :db effect will be produced and swapped into app-db only once
>
> -- https://github.com/status-im/swarms/issues/31#issuecomment-346345981

This is in fact incorrect. Re-frame, ever since 2015 (so before the original
discussions in 2017) uses a concept of batching to process events, which means
subscriptions won't re-run after every dispatched event, and thus components
won't re-render either. Re-frame is smarter than that.

> groups of events queued up will be handled in a batch, one after the other,
> without yielding to the browser (previously re-frame yielded to the browser
> before every single event).
>
> -- 39adca9367/docs/releases/2015.md (050--2015-11-5)

Here's a practical example you can try in a shadow-cljs :mobile REPL to see the
batching behavior in practice.

```clojure
;; A dummy event that toggles between DEBUG and INFO levels.
(re-frame/reg-event-fx :dummy-event
  (fn [{:keys [db]}]
    {:db (update-in db
                    [:profile/profile :log-level]
                    (fn [level]
                      (if (= "DEBUG" level)
                        "INFO"
                        "DEBUG")))}))

(def timer
  (js/setInterval #(re-frame/dispatch [:dummy-event])
                  50))

;; 1. In component status-im.ui.screens.advanced-settings.views/advanced-settings,
;; add a print call to see when it's re-rendered by Reagent because the
;; subscription :log-level/current-log-level will be affected by our dummy event.
;;
;; 2. Also add a print call to the subscription :log-level/current-log-level to
;; see that the subscription does NOT re-run on every dispatch.

;; Remember to eval this expression to cancel the timer.
(js/clearInterval timer)
```

If you run the above timer with 50ms interval, you'll see a fair amount of
batching happening. You can infer that's the case because you'll see way less
than 20 print statements per second (so way less than 20 recomputations of the
subscription, which is the max theoretical limit).

When the interval is reduced even more, to say 10ms (to simulate lots of
dispatches in a row), sometimes you don't see a single recomputation in a 5s
window because re-frame is too busy processing events.

This shows just how critical it is to have event handlers finishing as soon as
possible to relinquish control back to the UI thread, otherwise responsiveness
is affected. It also shows that too many dispatches in a row can be bad, just as
big event handlers would block the batch for too long. You see here that
dispatching events in succession does NOT cause needless re-computations.

Of course there's an overhead of using re-frame.core/dispatch instead of calling
a Clojure function, but the trade-off is clearly documented: the more we
process in a single event, the less responsive the app may be because re-frame
won't be able to relinquish control back to the UI thread. The total time to
process the batch increases, but re-frame can't stop in the middle compared to
when different dispatches are used.

Thus, I believe this rf/merge pattern is harmful as a default practice in an
environment such as ours, where it's desirable end-users feel a snappy RN app. I
actually firmly believe we can improve the app's responsiveness by not
coalescing events by default. We're also preventing status-mobile from taking
the most advantage from future improvements in re-frame's scheduler. I can
totally see us experimenting with other algorithms in the scheduler to best fit
our needs. We should not blindly reduce the number of events as stated here
https://github.com/status-im/status-mobile/pull/2634#discussion_r155243127.

Solution: only coalesce events into one pile when it's strictly desirable to
atomically update the app db to avoid inconsistencies, otherwise, let the
re-frame scheduler do its job by using fx, not rf/merge. When needed, embrace
*eventual app db consistency* as a way to achieve lower UI latency, i.e. write
fast and short events, intentionally use :dispatch-later or other timing effects
to bend the re-frame's scheduler to your will.

There's another argument in favor of using something like rf/merge which I would
like to deconstruct. rf/merge gives us a way to reuse computations from
different events, which is nice. The thing here is that we don't need rf/merge
or re-frame to reuse functions across namespaces. rf/merge complects re-frame
with the need to reuse transformations.

Instead, the solution is as trivial as it gets, reuse app db "transformers"
across events by extracting the logic to data store namespaces
(src/status_im/data_store). This solution has the added benefit of not causing
cyclic dependency errors.

### Problem 4: Clojure's language server doesn't understand code under rf/defn

Nowadays, many (if not most) Clojure devs rely on the Clojure Language Server
https://github.com/clojure-lsp/clojure-lsp to be more effective. It is an
invaluable tool, but it doesn't work well with the macro rf/defn, and it's a
constant source of frustration when working in event namespaces. Renaming
symbols inside the macro don't work, finding references, jumping to local
bindings, etc.

Solution: don't use rf/defn, instead use re-frame's reg-event-fx function and
clojure-lsp will understand all the code inside event handlers.

### Problem 5: Unit tests for events need to "test the world"

Re-frame's author strongly recommends testing events that contain non-trivial
data transformations, and we do have many in status-mobile (note: let's not
confuse with integration tests in status_im/integration_test.cljs). That, and
non-trivial layer-3 subscriptions should be covered too. The reasoning is that
if we have a well developed and tested state layer, many UI bugs can be
prevented as the software evolves, since the UI is partially or greatly derived
from the global state. See re-frame: What to Test?
39adca9367/docs/Testing.md (what-to-test).
See PR Introduce subscription tests
https://github.com/status-im/status-mobile/pull/14472, where I share more
details about re-frame's testing practices.

When we use rf/merge, we make unit testing events a perennial source of
frustration because too many responsibilities are aggregated in a single event.
Unfortunately, we don't have many devs in the team that attempted to write unit
tests for events to confirm my claim, but no worries, let's dive into a real
example.

In a unit test for an event, we want to test that, given a cofx and args, the
event handler returns the expected map of effects with the correct values
(usually db transformations).

Let's assume we need to test the following event. The code is still using the
combo rf/defn & rf/merge.

```clojure
(rf/defn accept-notification-success
  {:events [:activity-center.notifications/accept-success]}
  [{:keys [db] :as cofx} notification-id {:keys [chats]}]
  (when-let [notification (get-notification db notification-id)]
    (rf/merge cofx
              (chat.events/ensure-chats (map data-store.chats/<-rpc chats))
              (notifications-reconcile [(assoc notification :read true :accepted true)]))))
```

As you can see, we're "rf/merging" two other functions, namely ensure-chats and
notifications-reconcile. In fact, ensure-chats is not registered in re-frame,
but it's 99% defined as if it's one because it needs to be "mergeable" according
to the rules of rf/merge. Both of these "events" are quite complicated under the
hood and should be unit tested on their own.

Now here goes the unit test. Don't worry about the details, except for the expected output.

```clojure
(deftest accept-notification-success-test
  (testing "marks notification as accepted and read, then reconciles"
    (let [notif-1          {:id "0x1" :type types/private-group-chat}
          notif-2          {:id "0x2" :type types/private-group-chat}
          notif-2-accepted (assoc notif-2 :accepted true :read true)
          cofx             {:db {:activity-center {:filter        {:type types/no-type :status :all}
                                                   :notifications [notif-2 notif-1]}}}

          expected {:db         {:activity-center {:filter        {:type 0 :status :all}
                                                   :notifications [notif-2-accepted notif-1]}
                                 :chats           {}
                                 :chats-home-list nil}
                    ;; *** HERE ***
                    :dispatch-n [[:activity-center.notifications/fetch-unread-count]
                                 [:activity-center.notifications/fetch-pending-contact-requests]]}
          actual   (events/accept-notification-success cofx (:id notif-2) nil)]
      (is (= expected actual)))))
```

Notice the map has a :dispatch-n effect and other stuff inside of it that are
not the responsibility of the event under test to care about. This happens
because rf/merge forces the event handler to compute/call everything in one go.
And things get MUCH worse when you want to test an event A that uses rf/merge,
but A calls other events B and C that also use rf/merge (e.g. event
:profile.login/get-chats-callback). At that point you flip the table in horror
😱, but testing events and maintaining them should be trivial.

Solution: Use re-frame's `fx` effect.

Here's the improved implementation and its accompanying test.

```clojure
(defn accept-notification-success
  [{:keys [db]} [notification-id {:keys [chats]}]]
  (when-let [notification (get-notification db notification-id)]
    (let [new-notifications [(assoc notification :read true :accepted true)]]
      {:fx [[:dispatch [:chat/ensure-chats (map data-store.chats/<-rpc chats)]]
            [:dispatch [:activity-center.notifications/reconcile new-notifications]]]})))

(re-frame/reg-event-fx :activity-center.notifications/accept-success accept-notification-success)

(deftest accept-notification-success-test
  (testing "marks notification as accepted and read, then reconciles"
    (let [notif-1          {:id "0x1" :type types/private-group-chat}
          notif-2          {:id "0x2" :type types/private-group-chat}
          notif-2-accepted (assoc notif-2 :accepted true :read true)
          cofx             {:db {:activity-center {:filter        {:type types/no-type :status :all}
                                                   :notifications [notif-2 notif-1]}}}

          ;; *** HERE ***
          expected {:fx [[:dispatch [:chat/ensure-chats []]]
                         [:dispatch [:activity-center.notifications/reconcile [notif-2-accepted]]]]}
          actual   (events/accept-notification-success cofx [(:id notif-2) nil])]
      (is (= expected actual)))))
```

Notice how the test expectation is NOT verifying what other events do (it's
actually "impossible" now). Using fx completely decouples events and makes
testing them a joy again.

### Problem 6: Unordered effects

status-mobile still uses the legacy way to describe the effects map, which has
the problem that their order is unpredictable.

> Prior to v1.1.0, the answer is: no guarantees were provided about ordering.
> Actual order is an implementation detail upon which you should not rely.
>
> -- 39adca9367/docs/Effects.md (order-of-effects)

> In fact, with v1.1.0 best practice changed to event handlers should only
> return two effects :db and :fx, in which case :db was always done first and
> then :fx, and within :fx the ordering is sequential. This new approach is more
> about making it easier to compose event handlers from many smaller functions,
> but more specificity around ordering was a consequence.
>
> -- 39adca9367/docs/Effects.md (order-of-effects)

### Problem 7: Usage of deprecated effect dispatch-n

We have 35 usages, the majority in new code using dispatch-n, which has been
officially deprecated in favor of multiple dispatch tuples in fx. See
39adca9367/docs/api-builtin-effects.md (L114)

### Problem 8: Complexity 🧙‍♂️

Have you ever tried to understand and/or explain how rf/merge and rf/defn work?
They have their fare share of complexity and have tripped up many contributors.

This is not ideal if we want to create a project where contributors can learn
re-frame as quickly as possible. Re-frame is already complicated enough to grasp
for many, the added abstractions should be valuable enough to justify.

Interestingly, rf/merge is a stateful function, and although this is not a
problem in practice, it is partially violating re-frame's spirit of only using
pure functions inside event handlers.

### Problem 9: Using a wrapping macro rf/defn instead of global interceptors

When rf/defn was created inside status-mobile, re-frame didn't have global
interceptors yet (which were introduced 3+ years ago). We no longer have this
limitation after we upgraded our old re-frame version in PR
https://github.com/status-im/status-mobile/pull/15997.

Global interceptors are a simple and functional abstraction to specify functions
that should run on every event, for example, for debugging during development,
logging, etc. This PR already shows this is possible by removing the wrapping
function utils.re-frame/register-handler-fx without causing any breakage.

## Conclusion

By embracing re-frame's best practices for describing effects
39adca9367/docs/FAQs/BestPractice.md (use-the-fx-effect),
we can solve long standing issues that affect every contributor at different
levels and bring the following benefits:

- Simplify the codebase.
- Bring back the DX we all deserve, i.e. Clojure Language Server and clj-kondo
  fully working in event namespaces.
- Greatly facilitate the testability of events.
- Give devs more flexibility to make the app more responsive, because the new
  default would not coalesce events, which in turn, would block the UI thread
  for shorter periods of time. At least that's the theory, but exceptions will
  be found.

The actions to achieve those benefits are:

- Don't use the macro approach, replace rf/defn with
  re-frame.core/reg-event-fx.
- Don't use rf/merge, simply use re-frame's built-in effect :fx.
- Don't call event handlers as normal functions, just as we don't directly call
  subscription handlers. Use re-frame's built-in effect :fx.

## How do we refactor the remainder of the code?

Some numbers first:

- There are 228 events defined with rf/defn in src/status-im2/.
- There are 34 usages of rf/merge in src/status_im2/.

## Resources

- Release notes where fx was introduced in re-frame:
  39adca9367/docs/releases/2020.md (110-2020-08-24)
2023-10-05 16:11:45 -03:00
Siddarth Kumar 7043e03dd4
Enter scan code tab in Syncing (#16852)
* feat: add enter scan code view

This commit adds the enter sync code UI.

Fixes : #16062

Design link : https://www.figma.com/file/V6nlpAWIf2e1XU8RJ9yQPe/Syncing-for-Mobile?node-id=675%3A179729&mode=dev

Review notes :
We do not make use of quo2/input here because currently that component does not allow stretching to fit in sync code which spans upto 3 lines.
2023-10-05 21:05:54 +05:30
Volodymyr Kozieiev b534b33a09
Doc updated with instruction how to view db content (#17535)
* Doc updated with instruction to view db content
2023-10-04 19:29:55 +01:00
omahs 69df5a7e2f
fix: typos (#17484) 2023-10-04 16:54:32 +02:00
Volodymyr Kozieiev 08689e568a
Links to recorded meetings added to docs (#17399) 2023-09-22 16:42:58 +01:00
frank 72b8979c79
add doc: How to catch crash on ios use xcode (#17278)
* add doc: How to catch crash on ios use xcode

* addressed feedback from @ilmotta and @siddarthkay
2023-09-15 12:04:11 +08:00
Flavio Fraschetti 3d63854b03
Revise dev guidelines for code deprecation (#17177)
### Commit Summary
This pull request updates our coding guidelines to include a process for marking deprecated functions using metadata in ClojureScript.
2023-09-07 17:00:02 +01:00
diana 5e3255ea57
Docs update: required e2e 2023-09-01 15:26:26 +02:00
Jamie Caprani 4d7b1baace
chore: add Alex to wallet team (#17024) 2023-08-24 13:11:22 +02:00
diana e186f513ed
Add design review to pipeline process doc 2023-08-18 15:54:42 +02:00
Andrea Maria Piana d988296ecf
Add next in line for communities 2023-08-16 12:16:32 +01:00
Andrea Maria Piana dc8a09ce75
Update wallet team members 2023-08-16 12:16:08 +01:00
Andrea Maria Piana 9eaee8864c
Update status-go docs 2023-08-14 10:23:12 +01:00
Icaro Motta 6550bc058d
Document quo best practices and guidelines (#16901) 2023-08-09 11:32:36 -03:00
Icaro Motta b9890a9d44
Upgrade shadow-cljs and ClojureScript (#15417)
This commit upgrades Shadow CLJS from 2.11.16 (released on Feb/21) to latest
2.25.0 (Jul/23), so ~1.5 years worth of upgrades. By upgrading shadow we
can finally use the latest major Clojure version 1.11.x.

Why upgrade shadow?

- Shadow CLJS controls the ClojureScript version we can use. In order to use the
  latest major Clojure version we must upgrade Shadow CLJS.

- Shadow CLJS releases new versions very frequently, and if you take a look at
  its changelog https://github.com/thheller/shadow-cljs/blob/master/CHANGELOG.md, you'll see
  it had tons and tons of bug fixes over the years. I hope some of them help
  improve the DX for certain contributors who recently reported issues with
  it.

- Clojure 1.11 brings new features, bug fixes and even performance improvements
  (although I think the performance mostly impacts Clojure on the JVM). See the
  changelog https://github.com/clojure/clojure/blob/master/changes.md#changes-to-clojure-in-version-1110

Things that can be beneficial to us, or are interesting nonetheless:

- New :as-alias to be used in require, which is like :as but does not require
  the namespace to load. This means namespaced keywords using :as-alias can't
  cause circular dependency errors. This feature would very useful if we used
  namespaced keywords, but we don't, so...
  https://github.com/clojure/clojure/blob/master/changes.md#22-as-alias-in-require
- New macros run-test and run-test-var to run single test with fixtures and
  report.
- New iteration function, useful for processing paginated data.
  https://www.abhinavomprakash.com/posts/clojure-iteration/
- New update-keys function: applies a function to every key in a map.
- New update-vals function: applies a function to every value in a map.

Examples for update-vals and update-keys. They should perform better than the
common reduce-kv approach since they use a transient data structure.

    (let [m {:a 1 :b 2}]
      (update-vals m inc)) ; => {:a 2, :b 3}
    
    (let [m {:a 1 :b 2}]
      (update-keys m name)) ; => {"a" 1, "b" 2}

Why change namespaces within __tests__ directories?

Any namespace with the word --tests-- throws an error, like the one below. I
didn't bother investigating why, so I changed the guidelines to reflect the new
convention. It's probably related to the double dashes in the name.

    Namespace quo2.components.dividers.--tests--.divider-label-component-spec has a
    segment starting with an invalid JavaScript identifier at line 1
2023-07-28 13:40:54 -03:00
Jamie Caprani 4801342a42
chore: add doc on pixel perfection (#16539) 2023-07-12 03:31:14 -07:00
Jamie Caprani 9ac14b6fac
chore: update guidelines for figma api to match quo2 component api (#16454) 2023-07-10 02:45:39 -07:00
Icaro Motta 19ca8e28a5
Lint and fix missing trailing newlines (#16445)
Apply the Clojure Style Guide recommendation to end files with proper lines
(having a trailing newline character). See
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_206
2023-07-04 19:40:13 +00:00
Mohamed Javid ce82e87cd0
Update broken `Status development` link in `ide-setup.md` (#16391)
This commit updates the broken link of `Status development` in the `Start and connect the REPL` section of `ide-setup.md`.

Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
2023-06-26 16:43:58 +05:30
Andrea Maria Piana 398810066d
Add team member to decision (#16336) 2023-06-23 11:36:07 +02:00