DEV Community

Joe Avila
Joe Avila

Posted on • Updated on

Smashing my first bug

Contribute to open source they said, it's a great way to break into tech they said.

A lot easier said than done. I have been developing on the modern web for about a year. I used to code all the time back in the early 2000's when vanilla Javascript, HTML, and CSS was the law of the land. Things have changed a lot since then. Just like learning modern web development, getting involved in open source can feel like drinking from a fire hose.

Open Source Background

I haven't made any meaningful contributions to projects. I helped Joe Previte with a styling bug on his portfolio, but he already had the solution coded out.

I spent 6 months on a contract for a consulting firm where I got neck deep in a React/Typescript + GraphQL/NodeJS application. That really boosted my confidence in reading others code, and reading very large code files. That experience made me feel more prepared to jump into forem's codebase.

About This Bug

"Unable to load Comment Subscription component" #12545

Describe the bug

image

It's been happening quite often recently for me but I can't really tell how exactly this should be reproduced. It does work after refreshing the page, but then when I go to another post it's broken again.

Also, this is what's happening in the console:

image

To Reproduce

See above.

Expected behavior

Fo this component to work properly :)

Screenshots

See above.

Desktop (please complete the following information):

n/a

Smartphone (please complete the following information):

n/a

There was a console error thrown any time a text editor component was rendered after the first time. So if you visited an article, left and went to a different article then it would say a variable, subscribeBtn, had already been declared.

How I Smashed It

Fix "Unable to load Comment Subscription component" #13694

What type of PR is this? (check all applicable)

  • [ ] Refactor
  • [ ] Feature
  • [x] Bug Fix
  • [ ] Optimization
  • [ ] Documentation Update

Description

This PR fixes the console error described in the issue by changing the const declaration to var. I was unable to replicate the UI bug without repro steps.

The changePage function in base.js.erb sending over the user_subscription_tag when it's called, and declaring the subscribeBtn as a const was causing the error. I'm not familiar with liquid tags, but it seems that the other liquid tags that use JS declare variables with var instead of ES6.

Related Tickets & Documents

Resolves #12545

Added tests?

  • [ ] Yes
  • [ ] No, and this is why: please replace this line with details on why tests have not been included
  • [x] I need help with writing tests

[Forem core team only] How will this change be communicated?

Will this PR introduce a change that impacts Forem members or creators, the development process, or any of our internal teams? If so, please note how you will share this change with the people who need to know about it.

  • [ ] I've updated the Developer Docs and/or Admin Guide, or Storybook (for Crayons components)
  • [ ] I've updated the README or added inline documentation
  • [ ] I will share this change in a Changelog or in a forem.dev post
  • [ ] I will share this change internally with the appropriate teams
  • [ ] I'm not sure how best to communicate this change and need help
  • [ ] This change does not need to be communicated, and this is why not: please replace this line with details on why this change doesn't need to be shared

[optional] Are there any post deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

alt_text

Reflection

My biggest challenge in contributing to open source is believing in myself. Before asking to be assigned to the bug, I went into the code base and searched out the identifying variable. Once I found it, I had no idea what was going on or what the fix was but I knew it was within my grasp. Before I could think myself out of the challenge, I asked to be assigned.

I'm really glad I signed up for it because I was forced to play around in someone else's code base using a tool I'm not I'm not super familiar with: liquid tags. But my knowledge of javascript and googling powers helped me solve it.

Someone had declared a variable in Dev's text editor component with const. When a user (you) switched pages (to edit a post or comment, or to a new article) the entire bundle was being sent again. Because the original value was declared with const the application was throwing an error saying there was already a variable with the name subscribeBtn present.

Suggestions for DEV Community Bug Smash

The bin/startup command didn't work for me. Couldn't get foreman to run properly. I just ran the three commands in the that Procfile in separate terminal windows and that worked fine. Never looked into the foreman issue.

Top comments (5)

Collapse
 
rafavls profile image
Rafael

My biggest challenge in contributing to open source is believing in myself.

I can relate to that rush/fear of contributing to open source projects. Thankfully forem's reviewers/administrators were nothing but nice in my case. Keep up the good work Joe πŸ‘πŸ½

Collapse
 
shubhamk profile image
Shubham Kamath

β€œ But my knowledge of javascript and googling powers helped me solve it.”

This is important for anyone starting with development. Focus on basics and try to understand solutions written on internet.

Amazing work Joe!

Collapse
 
siddharthshyniben profile image
Siddharth

Isn't the command bin/setup?

Collapse
 
mono13th profile image
Moch. Sudharmono

Seems nice, I want to contribute to open source too.
I'll try to read the code base from github repo

Collapse
 
itsnikhil profile image
Nikhil Taneja • Edited

I really liked whatever you mentioned in reflection part.
I think this bug is still there πŸ˜… (attached a screenshot)
dev-to-uploads.s3.amazonaws.com/up...