DEV Community: Tue

Socket event listener fires multiple times in React component

Tue — Mon, 27 Jun 2022 03:53:36 +0000

Here is something I've learned doing my side project that I think it's fundamental but also often overlooked.

Some context

The project simply contains 2 major parts:

Front-end: React and Materials UI
Back-end: REST API using Express, Typescript, Firebase and Socket.io

My goal was to inform the front-end when data is updated using Socket.io so that it would try to fetch data again.

Incorrect attempts

I had these incorrect code snippets that listen to socket event and fetch data again

My first attempt was this, whenever there's any re-render, socket.on('USER_JOINED') would registers the same callback function passed into it so once the event USER_JOINED is emitted, the same callback will fire multiple times while we only need it to execute once to reduce api calls and performance obviously.

const Expense = () => {
  const [initialValues, setInitialValues] = useState(null);
  const [expense, setExpense] = useState(null);
  const { socket, toLogIn } = useContext(SWContext);
  // ...
  socket.on('USER_JOINED', (socketData) => {
    // fetch data again
  });
}

My second attempt was to register the callback once when the component is mounted but I still experienced multiple callback executions. It's because even after the component is unmounted, the callback is still registered with socket (I use one single instance of socket (Singleton pattern)). And I wouldn't have access to new state if the state was updated.

  useEffect(() => {
    socket.once('USER_JOINED', (socketData) => {
      // fetch data again
    });
  }, []);

Solution

This is what works for me so far. I register a socket event handler and a clean up socket.off every time expense changes. This way there's only one socket event handler being called at a time and detached when not needed

  useEffect(() => {
    socket.once('USER_JOINED', (socketData) => {
      // fetch data again
    });
    return socket.off('USER_JOINED');
  }, [expense]);

I imagine this practice can also apply for similar situations like window.addeventlistener()

Take a look at my project if you're curious https://github.com/TueeNguyen/SplitWise3

3.0 release, work is not yet done

Tue — Sat, 23 Apr 2022 04:15:41 +0000

I exclusively worked on this issue this week #3464.

Update Feed in `parser` to use Supabase:

I basically removed all invalid and flagged feed from src/api/parser/utils/storage and create functions that would take care of invalidating/flagging feeds using Supabase in src/api/parser/utils/supabase.js. Looking back, it wasn't that hard but it was quite lengthy.

The difficulty was that this function uses isFlagged function to check flagged feed and now it talks to Supabase instead of Redis which made unit test not pass without mocking it.

  addFeed: async (feed) => {
    // Check if feed being added already exists in flagged feeds set
    // If it is, do nothing
    if (await isFlagged(feed.id)) return;

    const key = createFeedKey(feed.id);
    await redis
      .multi()
      .hset(
        key,
        'id',
    // ...
  },

So I literally created a mock for Supabase database though I had a problem with telling Jest to use the mock module.

Originally, I imported the mock this way

jest.mock('../src/utils/supabase');
const { __setMockFeeds, __resetMockFeeds } = require('../src/__mocks__/utils/supabase');

So, __setMockFeeds would add the array to the actual module => not the mock factory that Jest uses. Those 2 are different javascript objects and Jerry helped me understand this.

// Correct way to mock
jest.mock('../src/utils/supabase');
const { __setMockFeeds, __resetMockFeeds } = require('../src/utils/supabase');

I also had to reworked the parser e2e test, now it looks even simpler. For some reason, I kept forgetting to do pnpm migrate and I was stuck on the test for a while. I managed to make it work a few hours before 3.0 release haha.

However, work is not yet done, Supabase doesn't really work in production, parser can't really talk to the database, I hope to fix this soon and earn myself some peace of mind.

Pre 3.0

Tue — Sat, 23 Apr 2022 02:55:43 +0000

The closer we approaching release 3.0, the more we try to narrow the scope of work and honestly I like how each of us knows what area of focus to take charge on and trim down the excess issues.

My tasks for this release again are mostly on parser service:

Remove the src/backend
Update invalid/flag feed functions to use Supabase
Update docs after removing src/backend

But let's focus on what I did the week prior to 3.0 release

Removing the src/backend:

One of the problem I came across was that we passed in our staging SUPABASE_URL and development SERVICE_ROLE_KEY to parser, the pair would never work and we'd never get staging SERVICE_ROLE_KEY as it is a secret. I had to change the pair so that the client could work locally, this created another challenge.
The reason we used staging url was Duke suggestion, he wanted to use the feeds from staging database instead of seeding feeds every time a developer spins up the services locally. Duke quickly wrote a solution to this in #3541 saving us from lengthy setup steps.

This wasn't an easy task, I had to do a lot of detective work, I had to find out which microservice or module that used the legacy backend, luckily there weren't much. Removing dependencies was mundane too, I had to figure out which dependency in was peer dependency in the root package.json.

Deploying the code to production made me a bit anxious, it did break production because I did not consider the nginx proxy config for legacy backend so nginx kept breaking. See those locations removed in #3550.

3.0.0-alpha.0

Tue — Sat, 09 Apr 2022 02:30:20 +0000

This release went quite smooth and FAST thanks to the recent docker changes, now we're pulling docker images instead of building them.

Improve parser's e2e test:

The problem was I loaded real feeds into the feedQueue and made it download real resource through network requests. This approach wasn't correct and prone to random failures. David suggested me using the resources from test-web-content. Looking back, I think I need to create another xml feed, these feeds below are duplicate. PR #3773

const valid = [
      {
        author: 'Tue Nguyen',
        url: 'http://localhost:8888/feed.xml',
      },
      {
        author: 'Antonio Bennett',
        url: 'http://localhost:8888/feed.xml',
      },
    ];

And for invalid feeds I just gave the queue non-existent feed urls

const invalid = [
      {
        author: 'John Doe',
        url: 'https://johnhasinvalidfeed.com/feed',
      },
      {
        author: 'Jane Doe',
        url: 'https://janehasinvalidfeed.com/feed',
      },
    ];

Change parser to pull feeds from Supabase:

I added SUPABASE_URL & ANON_KEY to parser container for it to connect to the database and wrote a function that returns all feeds. PR #3363

  async getAllFeeds() {
    const { data: feeds, error } = await supabase.from('feeds').select('wiki_author_name, url');
    if (error) {
      logger.warn({ error });
      throw Error(error.message, "can't fetch feeds from supabase");
    }
    const formattedFeeds = feeds.map((feed) => ({
      author: feed.wiki_author_name,
      url: feed.url,
    }));
    return formattedFeeds;
  },

Some pieces of the tests had to be modified and wiki parsing code was removed.

Remove src/backend:

This one is huge but at least I didn't have to write new code 😅. Most of the work was to determine which part of the legacy backend was still used by other services. Jerry helped me a lot with this PR, we went through files and found that API_URL was still passed to services though, it had only one use in /src/web/app/src/pages/_document.tsx.
The PR does work but there's still work to be done and tested.

What I've learned doing reviews or looking at other PRs 😛:

SWR react hook from PR #3240
Interesting way to solve the back up issue PR #3405
Use ORM to manage database change PR #3418

2nd time being Sheriff

Tue — Tue, 05 Apr 2022 14:01:55 +0000

Sheriff duties:

Last week, my sheriff duties were a bit different than usual. Aside from going over issues/PRs, assigning issues to people, I worked with Hung to group issues into different areas that we wanted to focus on for 3.0 alpha in the wiki. Moreover, with the help of other colleagues in the Thursday triage meeting, we went over the list of issues again and decided which one to go or stay in the project's milestone. I saw my growth in that session, I didn't know much about Telescope just 3 months ago and last week I knew which part of the project to be prioritize, shifted even to lower level in some parts I didn't work on, I gained the knowledge from doing reviews.

My contribution:

I didn't do much in terms putting more code into the repo but I helped people move their code by doing reviews. One of the highlights last week was

We finally deployed Supabase to production, hiding it behind an auth wall. This required changes to docker and nginx which were mostly done by David and Kevan. I need to read up on Kevan's work to understand nginx implementation.
We added a secret.env file to live in staging and production
Seeding feeds script to supabase
We had some hiccups with the oAuth configs but they were resolved quickly

2.9 was exciting

Tue — Sat, 26 Mar 2022 00:19:19 +0000

This week I have the chance to spend more time and mind on Telescope since I don't have to worry about looking for an internship any more and this release has been productive and full of knowledge.

What have I done?

My parser tests are finally done, thank you David for being patient with this task. I'm pretty sure that I close all connections that are open during the e2e test but something still hangs, I suspected that it comes from [Bull](https://github.com/OptimalBits/bull) but it was hard to figure out what it actually was, therefore, I decided to --forceExit[https://jestjs.io/docs/cli]

After the tests were done, I dockerized the parser service, it wasn't too hard as Kevan had done similar Dockerfile refactoring PRs and I had other images as an example. But I still took some time to understand what's going in those docker files.

Multi stage build, I guess it speeds up the process, it makes it easier to control data you want to pass to the image.
User deterministic docker base image tags to achieve more stable build
Don't execute Node.js app as root but use a node (lower privilege user) to improve security and the command --chown

How I helped others and learned as well?

Roxanne put an autocomplete for authors PR which was very interesting, people were really responsive, it has 42 conversation in a few hours :D. It took me a while to learn what is n-gram, edge n-gram, how we need to the n-gram analyzer to index the database. I pretty much just raised some questions in the PR. Elasticsearch really is complicated, though Roxanne was able to answer most questions, kudo to you. This is also useful for my other project, it came just in time :D

Francesco opened a PR to implement copy button, I found it very interesting so I tried to play and make it work, I made some progress and sent a PR for Francesco to take some inspiration.

I also reviewed some dependency discovery service pieces, I'm not exactly sure how it works yet, will need to talk to Jerry to help him with storing data through parser. Other than these PRs, I left my reviews in different PRs too.

There were a lot of Docker movements the last few days, I want to catch up to them but I guess I need more fundamentals first

Narrowing the work scope

Tue — Thu, 17 Mar 2022 03:47:09 +0000

We're only 2 releases away from 3.0 so we need to narrow the vision and focus on what we want to ship in 3.0

What part do I own?

Well, I've been working on Parser exclusively, it's moving slower than I thought and I've hit a road block where I can't get the Parser service to shutdown gracefully which affects Jest to not shutdown cleanly.
I've also looked at and attempted implementing Supabase so I'll help Telescope to bring it to the front-end and rework Parser to use Supabase. To do this, I need to under the Docker pieces more essentially, we need to hide the Supabase studio behind Portainer (admin app for Docker containers).
There's also a PR I put up but there have been different opinions, let's see how we can solve it

Foreseeable roadblocks:

To this point, I'm not sure why my e2e test is not working properly, I think I'll ask a few people to join a call so that I can demonstrate the problem.
I might need some time with Docker but I think I'm catching up quite quickly
Communication between areas is actually very important and hard which is often overlooked, my Parser will be reworked to user Supabase, to serve Dependency Discovery's purpose.

Touching more Jest tests

Tue — Sun, 13 Mar 2022 04:22:13 +0000

What've I contributed to 2.8

I've had little impact this release. My parser service e2e test works but not perfect, it does test how valid and invalid feeds should be processed when added to the feedQueue but this service doesn't shutdown gracefully. There are a few open handles that hangs Jest:

setInterval()

  const timer = new Promise((resolve, reject) => {
    timerId = setTimeout(() => {
      reject(
        new Error(
          'Unable to connect to Elasticsearch. Use `MOCK_ELASTIC=1` in your `.env` to mock Elasticsearch, or install (see https://github.com/Seneca-CDOT/telescope/blob/master/docs/environment-setup.md)'
        )
      );
    }, DELAY);
  });

  const connectivity = new Promise((resolve) => {
    intervalId = setInterval(async () => {
      try {
        await checkConnection();
        resolve();
      } catch (error) {
        logger.info('Attempting to connect to elasticsearch...');
      }
    }, 500);
  });

  await Promise.race([timer, connectivity]);

  await clearInterval(intervalId);
  clearTimeout(timerId);

The result of Promise.race if all things are well should be a resolved Promise from connectivity, after which is cleared by clearInterval, somehow Jest still detects as open, this could be related to JavaScript event queue. I also tried using unref() following Tim Perry's blog post but to a avail.

logger.

Thanks to David's PR turning logger off during test, this problem went away

Redis open connections

Some Redis connections are still open after Jest finishes all tests, this was straightforward enough, I tried adding some code that keeps track of all open connections and close them after use and it worked but the logic already exist in Satellite, therefore, I wanted to reuse that code. Thanks to Francesco's work to merge Satellite to Telescope, adding anything to Satellite has become easier.

Modifying Satellite

I opened a PR to modify the stop() from Satellite class to close all database connections (ElasticSearch & Redis). However, I struggled more than I thought.

First of all I changed stop() to return an array of Promises, I used promisify to make server.close() return a Promise but it lost access to this.server in Satellite so I had to bind it with this.server instance.

    const { server } = this;

    const promisifiedClose = server
      ? promisify(server.close.bind(server))
      : () => Promise.resolve();

One of my mistakes was not to pay attention to this piece of code

afterEach(() => {
  service.stop()
});

Yes, the brackets are not needed, I should only return the Promise for Jest to wait.

However, there's still problems with the service not shutting properly after the Satellite's change 😕

Planning for release 2.8

Tue — Sun, 27 Feb 2022 02:47:53 +0000

This week, we talked to each other on what we were gonna work on for release 2.8.

My plan is to finish the parser service, I'm struggling a bit with e2e tests but (Roxanne)[https://github.com/rclee91] offered me some help with changing the mocked indexer to Elasticsearch mock, it became easier.

Writing e2e tests for parser:

Most unit tests are written already, I just ported those tests over to sr/api/parser, changed some imports and added a few small tests.

My approach is to clear the redis database first and add a few feeds (invalid, valid, youtube) to the queue to process using a custom mocked getWikiFeeds() and fetch posts service to compare the result when the queue is drained.

My mistake when trying to write e2e tests was that I tried to use mocked Redis, but e2e is where you test the flow of the application so mocking is not needed. I've got the first test to work but encountered this problem

Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

I then ran the test with --detectOpenHandles and found out that the code had a few Redis connections open, somehow logger is an open handle, such as

// src/api/parser/src/lib/queue.js
const client = Redis();
const subscriber = Redis();

// /src/api/parser/src/utils/storage.js
const { logger, Redis } = require('@senecacdot/satellite');
const redis = Redis();

I have something in mind, will update this blog when I find a solution!

Personal notes: I hope the people of Ukraine can stay safe, I have a few Ukrainian friends and I can't imagine being in their shoes, it hurts me when thinking about it. The world has become unpredictable in the last 2 year.

First time being Sheriff

Tue — Sat, 19 Feb 2022 16:49:50 +0000

Oh, not that Sheriff guys, this Sheriff. On the side note, I seriously need to add things I've learned to a list to remind myself.

Being sheriff

I was a lot of work for sure, I had to manage the repo, move issues/PRs forward, assign them to the right people, reorganize milestones, and I went even further reviewing most of the PRs submitted this week. Here's what I've learnt:

React Context through Diana's PR, and a chance to look at Abdul's PR on React custom hooks that I didn't look
How does Telescope use JWT to authenticate
How does auto deployment server inject variables that are only determined during run time
How do we use turbo repo in telescope, and plan to transform and manage Telescope as a monorepo

And of course, gained more knowledge on how Telescope works as a whole.

The hardest part wasn't reviewing work from other contributors (though it was hard reading and understanding them), it was organizing issues and PRs into correct milestones, there were a lot of them :D.

Breaking production

We can't seem to just have a zero in our version number 😃. There were a number of bugs but one of them was critical. We're changing the way both Satellite and Telescope use JWT because we've introduced Supabase to our project. Basically, we changed Satellite to use JWT_SECRET instead of SECRET but Joel's PR forgot to change the variables in env.staging and env.production which resulted in this error

  if (!options || !options.secret) throw new Error('secret should be set');
                                   ^

Error: secret should be set
    at module.exports (/home/node/app/node_modules/express-jwt/lib/index.js:20:42)
    at isAuthenticated (/home/node/app/node_modules/@senecacdot/satellite/src/middleware.js:8:10)
    at Object.<anonymous> (/home/node/app/src/routes/register.js:17:3)
    at Module._compile (node:internal/modules/cjs/loader:1103:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/home/node/app/src/routes/index.js:8:25)

My contribution:

I wrote a simple test for the dependency-discovery service which required me to refactor an export of the module to mock one
of the functions so that we wouldn't rely on the deps.txt file.

I managed to fix renovate bot issues and my parser service is very close to be merged. Oh, please Tue, remember to remove unnecessary dependencies next time you work on something similar 🙃.

5th week in

Tue — Sun, 13 Feb 2022 19:05:07 +0000

Being more involved:

I've been trying to be more involved in reading up people's work and reviewing PRs. I started out by testing the PR, reading the code and commented any questions I had, as Dave said it's also about spreading knowledge. It was challenging because some PRs can take long to read and understand but I did learn more especially with Duke's PRs regarding Supabase 2844 but couldn't leave any advice. Meanwhile, with less intensive PRs, I learned as I go and I actually helped clear around 7 PRs, wow.

Fixing Renovate bot not opening PRs:

I was assigned to fix Renovate bot not creating PRs. At first, I didn't know much about this bot, but I'm used to these situations, I just started reading up on it and the log. It took a while, but eventually I found this section in the Renovate Docs, it says that

branchConcurrentLimit
Limit to a maximum of x concurrent branches. 0 means no limit, null (default) inherits value from prConcurrentLimit.

While the log says

DEBUG: Reached branch limit - skipping branch creation(branch="renovate/fontsource-monorepo")

I suspected that there was a branch created by Renovate, and somehow it didn't get merged. And yes, I deleted that branch, Renovate started working again. However, there's still an issue with this bot, I have an idea how to fix it, will attempt it soon.

Parser service again 😅:

I've been gradually fixing parser service, it seems like there is some unnecessary code in it because I literally copied from backend. It seems like backend does expose a few routes in the web folder, I need to bring this up to Dave and Josue to understand what is expected from this service (does it expose any routes or does it simply just process feeds?), it affects the tests I'm writing. By the way, I came up with some ideas for e2e tests, I'm gonna finalize my thoughts first before bringing this up to people as well.

A list of things I've learnt:

The differences between next() and done()
PostgreSQL: Row level security, seed script, cascade delete
How JWT is gonna be used in telescope (need to wrap my head around this)
Dependency-discovery service (how it works)

What's down the road:

Sheriff this week
Helping Jerry with Dependency-discovery service, he got me interested in this
Making parser work (it seems like more work than I anticipated)
Writing tests for parser (in progress)
Dockerize parser
Changing parser so that we can work with Supabase
Twitch integration

Final progress on 2.6 release

Tue — Sun, 06 Feb 2022 02:22:51 +0000

Fixing showstopper bug:

A few days ago, I tried to start docker containers to debug this feed 304 issue, which we eventually closed because we couldn't reproduce the bug, but there were errors regarding mdx

./src/markdown-pages/about.md
Attempted import error: 'mdx' is not exported from '@mdx-js/react' (imported as 'mdx').

It was working just fine a few hours before, we were all taken by surprise by it. It came from front-end part of the code, since mdx is a dependency from the front-end. After changing versions of next and mdx, nothing seemed to work, we decided to take a look at their GitHub, turned out they had released a new version also a few hours before. We tried a few options based on the answer from this issue but only installing mdx-js/react worked for us. We got together on Teams to discuss and fix this, the fix was small but it was fun

Making parser service run locally:

I'm in charge of making parser service run and adding tests. Since, backend has changed a lot for the last few months, making parser service run requires some effort. I need to make this happen in 2.7 or else people are gonna add things to backend and I have to update it 😂.

I was trying to run parser by using normal node index.js but encountered 2 errors. Sometimes, Redis complained about connection being in subscriber mode so it couldn't do any CRUD operations

[ 2022-01-30 18:51:41.815 ] ERROR (20140 on DESKTOP-P67GFS3): Error queuing feeds
    err: {
      "type": "Error",
      "message": "Connection in subscriber mode, only subscriber commands may be used",
      "stack":
          Error: Connection in subscriber mode, only subscriber commands may be used
              at Redis.sendCommand (D:\Repos\telescope\src\api\parser\node_modules\ioredis\built\redis\index.js:633:24)
              at Redis.hgetall (D:\Repos\telescope\src\api\parser\node_modules\ioredis\built\commander.js:122:25)
              at getFeed (D:\Repos\telescope\src\api\parser\src\utils\storage.js:105:26)
              at byId (D:\Repos\telescope\src\api\parser\src\data\feed.js:157:24)
              at Array.map (<anonymous>)
              at Function.all (D:\Repos\telescope\src\api\parser\src\data\feed.js:182:28)
              at processTicksAndRejections (node:internal/process/task_queues:96:5)
              at async Promise.all (index 0)
              at async processAllFeeds (D:\Repos\telescope\src\api\parser\src\index.js:75:25)
    }

Or sometimes, it was just a plain unhandled error.

With the help of Josue, we found the cause to be in redis.js, the Redis connection. We were exporting an instance of Redis for queue.js to use and it didn't like that because client and subscriber have the same instance I think.

// queue.js
const Bull = require('bull');
const redis = require('./redis');
const { logger } = require('../utils/logger');

const client = redis;
const subscriber = redis;

// redis.js
const { Redis } = require('@senecacdot/satellite');
const redis = Redis();
module.exports = Redis;

It worked after the change below

// redis.js
const { Redis } = require('@senecacdot/satellite');
module.exports = Redis;

So for 2.6, I've managed to put these PRs 2716 2781 in.

Other than those major work I mentioned, I've been mostly talking to people and reading up stuff. I had 2 interviews this week which went really well, it occupied a piece of my mind so my progress has been quite slow.

DEV Community: Tue

Socket event listener fires multiple times in React component

Some context

Incorrect attempts

Solution

3.0 release, work is not yet done

Update Feed in parser to use Supabase:

Pre 3.0

Removing the src/backend:

3.0.0-alpha.0

Improve parser's e2e test:

Change parser to pull feeds from Supabase:

Remove src/backend:

What I've learned doing reviews or looking at other PRs 😛:

2nd time being Sheriff

Sheriff duties:

My contribution:

2.9 was exciting

What have I done?

How I helped others and learned as well?

Narrowing the work scope

What part do I own?

Foreseeable roadblocks:

Touching more Jest tests

What've I contributed to 2.8

Modifying Satellite

Planning for release 2.8

Writing e2e tests for parser:

First time being Sheriff

Being sheriff

Breaking production

My contribution:

5th week in

Being more involved:

Fixing Renovate bot not opening PRs:

Parser service again 😅:

A list of things I've learnt:

What's down the road:

Final progress on 2.6 release

Fixing showstopper bug:

Making parser service run locally:

Update Feed in `parser` to use Supabase: