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

refactor: convert analytics module to TypeScript/fp-ts, remove statistics animation on landing page #1361

Merged
merged 23 commits into from
Mar 16, 2021

Conversation

liangyuanruo
Copy link
Contributor

@liangyuanruo liangyuanruo commented Mar 12, 2021

Problem

Migrating Analytics client service to TypeScript

Addresses #1339

Remove statistics animation

Landing page statistics animation is not only tacky, but violates many principles of UI animation:

  1. It tests the patience of the user, taking around 4s to complete counting up, depending on the speed of the browser and device. The lag is also much greater at ~10s to complete on older browsers and devices,
  2. UX guidelines such as Material Design cite animations should take no more than 300ms.
  3. It is wholly unnecessary and only serves as a distraction from simply displaying the numbers

Solution

First, I removed the browser statistics animation entirely, moving the code for its retrieval to be resolved before the page is mounted. The Analytics client service was also rewritten in TypeScript.

Next, I created a new /analytics/statistics API endpoint as an additional optimisation to unify the three API calls to fetch the user, form, and submission counts respectively. Although speed gains ought be minimal due to HTTP/2 multiplexing, it does allow for the frontend and backend to share the new AnalyticStats type.

A fix was also put in MyInfo tests imports, which accidentally imported from the build folder definition instead of source - see 6a9c7c2.

Introducing fp-ts

I decided to introduce fp-ts to the module so that we may gain exposure from a wider set of functional programming paradigms beyond the small subset provided by fp-ts. The analytics module is well-suited as it is relatively small and standalone in nature.

Conversion from neverthrow to fp-ts has been relatively straightforward - Result<T, E> becomes Either<E, A> and ResultAsync<T, E> becomes TaskEither<E, A>, with the left and right type parameters swapping sides.

Imperative programming style is replaced with constructing a larger function via the pipe function, then executing it immediately, almost IIFE-style. The logical fork of an if-else statement is replaced with bimap, which accepts two map functions for the failure and success case of a TaskEither result. The utility function Result.combine to aggregate Results is replaced with sequence, which can convert TaskEither<E, A>[] to TaskEither<E, A[]>.

@liangyuanruo liangyuanruo marked this pull request as ready for review March 12, 2021 14:25
@liangyuanruo liangyuanruo requested a review from karrui March 12, 2021 14:25
@liangyuanruo liangyuanruo marked this pull request as draft March 14, 2021 09:30
@liangyuanruo liangyuanruo changed the title fix: remove statistics animation on landing page fix: convert analytics module to TypeScript/fp-ts, remove statistics animation on landing page Mar 14, 2021
@liangyuanruo liangyuanruo changed the title fix: convert analytics module to TypeScript/fp-ts, remove statistics animation on landing page refactor: convert analytics module to TypeScript/fp-ts, remove statistics animation on landing page Mar 14, 2021
- A for Array
- E for Either
- F for function
- TE for TaskEither
@liangyuanruo liangyuanruo marked this pull request as ready for review March 15, 2021 03:51
@liangyuanruo liangyuanruo requested a review from mantariksh March 15, 2021 08:56
@liangyuanruo liangyuanruo merged commit 8ce6479 into develop Mar 16, 2021
@liangyuanruo liangyuanruo deleted the fix/landing branch March 16, 2021 02:34
@karrui karrui mentioned this pull request Mar 23, 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.

2 participants