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
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:
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?
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.
Latest comments (5)
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...
Isn't the command
bin/setup?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 👏🏽
“ 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!
Seems nice, I want to contribute to open source too.
I'll try to read the code base from github repo