Open Source Contribution Update

Ryan Wilson
Nov 15, 2019 · 9 min read

As mentioned in my previous blog post, the assignment we were assigned was to file a PR for our internal and an external project. For our internal project our PR must be merged to count and for the external PR we must pick an issue that is larger than anything we did in Hacktoberfest. In my opinion this assignment was definitely more challenging than the previous assignment of submitting one blog post and one PR for Hacktoberfest.

Internal PR

For the Telescope project I created a basic email sender using Nodemailer which will send a message to a list of recipients with the desired message content. I really enjoyed to research and implement the different email senders to test to see which one is the most suitable including Nodemailer and SendGrid. By the end of that conversation, I decided to use Nodemailer because SendGrid could not send to the Seneca email servers as it refuses the connection.

This pull request consisted of many messages back and forth to change and add to my initial PR.

31 Messages

At first when I created this sender, it was used to send from a gmail account. This is a portion of initial code

const transporter = nodemailer.createTransport({
service: ‘gmail’,
auth: {
user: ‘’, // Email Name (.env variables must be used refer to issue#60)
pass: ‘’, // Email Pass (.env variables must be used refer to issue#60)
},
});

After discussions, environmental variables were added to the .env/sender file, promises were added, return type was changed from info to info.accepted for other issues and the service was switch to an SMTP server.

This is what the final code looks like

Step 1

exports.sendMessage = async function (receipiants, subjectMessage, message) {
return new Promise((resolve, reject) => {
const transporter = nodemailer.createTransport(‘SMTP’, {
host: process.env.NODEMAILER_SERVER,
port: 25,
secure: false,
auth: {
user: process.env.NODEMAILER_USERNAME,
pass: process.env.NODEMAILER_PASSWORD,
},});

Step 2
transporter.verify((error, success) => {
if (error) {
console.log(error);
} else if (success) {
console.log(‘Server is running properly’);
}});

Step 3
const mailOptions = {
from: process.env.NODEMAILER_USERNAME,
to: receipiants,
subject: subjectMessage,
html: message,
};

Step 4

transporter.sendMail(mailOptions, (err, info) => {
if (err) {
reject(err);
} else {resolve(info.accepted); }});});};

How it works

The above code is fairly simple,

  • Step 1 — Create the transporter which is used to connect the services
  • Step 2 — Used to verify the connection for the SMTP server
  • Step 3 — Message Content
  • Step 4 — Send the message with the message content and return a promise if it fails or succeeds

The tests for nodemailer are created in another issue — https://github.com/Seneca-CDOT/telescope/pull/198

Process

Nodemailer comes with really good documents as you can find a template for the code. The only process that is involved is which service you want to use whether its an email or SMTP. After you figure that out, you can configure some of the components such as which info to return when successful (example: info.accepted will return an array of recipients that received the email)

Experience

My experience creating this first pull request was pretty good as I learned as I made many different mistakes. The most important thing that I got out of this, is that sometimes it takes many days for others to respond which can make it slow to complete a task even if its small. When your relying on others so you can get the feature merged, it was frustrating when you have all of the approvals but your package.json has conflicts. This means that you must git pull upstream master, then rebase into your branch and get their approvals again which may take hours if others are inactive. I added an explicit variable into the Nodemailer transport but it does Travis Ci reject. In PR #227, I removed this explicit variable.

External PR

My external PR was from one of the projects that I completed a PR for on Hacktoberfest. The project that I choice was Darkreader and the issue was that the shortcuts text duplicate when you click on the them and click on the toggle buttons.

Issue

Below is an image of the issue that occurs.

Exact steps to replicate this issue.

Issue is on Firefox

  1. Click on the either “Toggle current site” or “Toggle extension”, this text will change to “…⌨”
  2. Click any of the buttons (toggle buttons, navigation buttons)
  3. The shortcut button that you clicked on should have duplicate text

Preparation

The code base for Darkreader was huge and I did not know where to start.

I started from the structure of the code. When you first grab the app from Github, you must run two commands NPM Install and NPM Run Debug.

The NPM Run Debug automatically generates two debug folders, one is for Firefox and the other is for Chrome. So i started to look around the debug folder created for Firefox. After searching, it seems like it will be in this popup index.js file as its an issue with the component on the UI page. My first impression is that this one file is huge as it contains 3000 lines of code, but its probably not that big compared to some of the other software that we use on a daily basis.

The default for these shortcuts are “Alt+Shift+A” and “Alt+Shift+D” so i tried to find the instances in this file.

Find instances of shortcuts

I put a breakpoint and started to walkthrough the program to try to understand the code.

First Function
Second Function
Third Function

After this function, It runs through many more functions before it actually sets the node in the below function

After hours of walking through the code, I still had no idea what was changing text. So i decided to play around with the code and stumbled across this function.

Now what does this do? Basically after clicking any elements on the UI it reinserts the nodes. I assume it checks to see if anything is mismatched and if it is then it insertNode.

So it reinserts the nodes, but does not actual set the text in this function. So maybe if it finds multiple instances of the Toggle text then do not reinsert and keep it the same.

I added this if statement which prevents the shortcut from being reinserted and it works. After I finished, I realized that you can not submit the files in any of the debug folders which means that I made zero progress in solving this issue after many hours.

I tried to find this insertNode function but it did not exist in the actual files. I found out that all their Node functions are generated by a Javascript framework called Malevic.js from the below image.

Back to the drawing board, I look through the source files to see if I can find anything for the shortcuts. Founds the shortcut folder and the index.tsx that comes with it.

I set a breakpoint to see what each function does

This function set the shortcut text and if you press a button down while your focused on it, you can change the shortcut

This is the off focus function, after you click off. The purpose is to set the text back to the original text.

Combining my knowledge from walking through the debug file and walking through current files. When you click on the shortcut and click a button, it sets the text to the original text in the onBlur() function and then in the matches function it adds onto the text message because node.textContent = “…⌨”

I played around for a few days in the onBlur function to see if i could catch to see if Toggle appears more than once, but the data coming into that function is inconsistent.

Data that comes back

Toggle extension Alt+Shift+D
Toggle extension Alt+Shift+D
Toggle extension Alt+Shift+DToggle extension Alt+Shift+D

After playing around with the onBlur() function it was impossible to find a solution because of the initial state and second state being “Toggle extension”.

After a while I found that every time it calls the InsertNode() function in the debug folder it calls this function

Solution

const shortcutClass = document.getElementsByClassName(“shortcut”);

const toggleCurrent = document.getElementsByClassName(“shortcut”).item(0);

const toggleExtension = document.getElementsByClassName(“shortcut”).item(1);

if (shortcutClass.length == 3) {

if (toggleCurrent.innerHTML.lastIndexOf(“Toggle”) > 5) {

toggleCurrent.innerHTML = toggleCurrent.innerHTML.slice(0, toggleCurrent.innerHTML.indexOf(“Toggle”, 1));

}

if (toggleExtension.innerHTML.lastIndexOf(“Toggle”) > 5) {

toggleExtension.innerHTML = toggleExtension.innerHTML.slice(0, toggleExtension.innerHTML.indexOf(“Toggle”, 1));

}}

Just a brief summary as this blog post is pretty long. I created three constant variables which represent the two shortcut buttons

It comes into this function on the initial creation which means that it inserts the nodes one by one onto the UI and also when it refreshes the UI in the matches function.

I check to see if the data coming back contains at least 2 instances of Toggle which means the data represents (Toggle extension Alt+Shift+DToggle extension Alt+Shift+D).

If 2 instances of text occur, it then slices the text to take the first instance and then sets the shortcut text.

Experience

This PR was a wild ride, it made me realize that a small bug can mean that you have to search through and understand a lot of the code. The most important lesson that I learned is that you should change the file and save it to see if you can make commits to it before spending too much time finding the solution. I really enjoyed to see the coding styles and also trying to understand how to code works. Its really hard to understand the code when a portion of what your working on it generated by a Javascript framework but after you understand enough of the code, it becomes easier. Walking through the code with the debugger is probably the best things that I did to prepare myself for this PR. I now understand most of the code that goes into the shortcuts on the UI and a general understanding of the entire UI which will come in use in future PR’s.

Links

External

Issue — https://github.com/darkreader/darkreader/issues/1621

Pull Request — https://github.com/darkreader/darkreader/pull/1732

Internal

Issue — https://github.com/Seneca-CDOT/telescope/issues/25

Pull Request — https://github.com/Seneca-CDOT/telescope/pull/93

Previous Blog Post

https://medium.com/@rwilson31/open-source-as-a-class-68c7434c83a1

Ryan Wilson

Written by

Hi my name is Ryan Wilson and these blogs are for a course called OSD600

Welcome to a place where words matter. On Medium, smart voices and original ideas take center stage - with no ads in sight. Watch
Follow all the topics you care about, and we’ll deliver the best stories for you to your homepage and inbox. Explore
Get unlimited access to the best stories on Medium — and support writers while you’re at it. Just $5/month. Upgrade