I like to read engineering and technology related articles shared by wonderful folks our various platform like Medium, Dev.to, Twitter, etc. One day while scrolling through my feed I stumbled upon #bugsmash program.
I looked into the open issues and picked Add eslint rule for formatting redundant curly braces in jsx #12643. During this time I was reading about ASTs, learning how linters work so I thought it would be a good issue to tackle.
About the bug
Add eslint rule for formatting redundant curly braces in jsx
#12643
This issue is to consider enabling the rule for react/jsx-curly-brace-presence in our eslint config. This could be set up to catch two formatting/code style issues, e.g.
<MyComponent type={ 'exampleType' }>{ 'Hello world' }</MyComponent>
Would be corrected to:
<MyComponent type="exampleType">Hello world</MyComponent>
It's a minor code style issue, but would make our JSX components more consistent and readable. We already have eslint-plugin-react as a dev dependency in the project, and could enable this rule as a warning initially.
NB: From a bit of investigation it seems Prettier can't do this for us which is why I'm suggesting eslint
The task was to be set up ESLint rule to catch the following formatting/code style issues in react
<MyComponent type={ 'exampleType' }>{ 'Hello world' }</MyComponent>
Would be corrected to:
<MyComponent type="exampleType">Hello world</MyComponent>
As, you can see below, I ran the linter on all JavaScript files and it found and fixed 1 issue here ->
How I Smashed It
#12643 Add eslint rule for formatting redundant curly braces in jsx
#14592
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Optimization
- [ ] Documentation Update
Description
Initially I thought I would have to build and publish my very own eslint plugin to solve this issue but later on I found out that such linting rule already exists. After merging this PR you can expect to see this linting check in action fixing where
<MyComponent type={ 'exampleType' }>{ 'Hello world' }</MyComponent>
Would be corrected to:
<MyComponent type="exampleType">Hello world</MyComponent>
As, you can see below, I ran the linter on all JavaScript files and it found and fixed 1 issue here -> https://github.com/itsnikhil/forem/commit/17508f12970621a323711cc5f08e0a67b7a7977b#diff-55437c4c12bdc7f18b73ceb37af6c27f95e647ef5ada26083c7fa53bd3730832R112
Related Tickets & Documents
This PR fixes https://github.com/forem/forem/issues/12643
QA Instructions, Screenshots, Recordings
You can run the following command and such linting violations will be marked as errors
yarn run lint:frontend
And with --fix, it will try to automatically fix such errors
yarn run lint:frontend --fix
UI accessibility concerns?
N/A
Added/updated tests?
- [ ] Yes
- [x] No, there are no unit tests for linting violations.. I have done manual testing but if there is some way please let me know.
[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've added an entry to
CHANGELOG.md - [ ] 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
What gif best describes this PR or how it makes you feel?
The solution was as easy as adding any other existing rule. Since eslint-plugin-react was already added as a dev dependency in the project, I followed the guide here https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-curly-brace-presence.md
// .eslintrc.js
rules: {
/*
Existing rules
*/
'react/jsx-curly-brace-presence': [
'error',
{ props: 'never', children: 'never' },
],
},
Before reaching to above solution, I explored https://astexplorer.net/ and how to work with rules. I also learned how to write my very own ESLint rule.
<MyComponent type={ 'exampleType' }>{ 'Hello world' }</MyComponent>
The above jsx in AST have following structure:
So, we can have a following function which apply on all JSXExpressionContainer using visitor pattern and check if typeof node.expression.value is a string then we can replace the expression with a fixer by replacing it with node.expression.raw
function (context) {
return {
JSXExpressionContainer(node) {
if (typeof node.expression.value === "string") {
context.report({
node,
message: "Do not use redundant curly braces",
suggest: [
{
desc: "Replace `{ 'string' }` with `'string'`",
fix: function (fixer) {
return [fixer.replaceText(node, node.expression.raw)];
},
},
],
});
}
},
};
}
You can see the whole thing in action
https://astexplorer.net/#/gist/b146b6ba734c554f51e4d5bb53023dda/e4077cfc126335c9468cca9eba01c675addf177d
Reflection
It was great experience building forem's source code and I like that the DEV team really appreciated my PR however small it was.
