Cleanup code base to be more readably and easier to maintain and contribute. #138
romainmenke
started this conversation in
General
Replies: 1 comment
-
I love this and I wholeheartedly agree with this, we've been going on with this already but having this already written will be definitely helpful. I think that rather than taking a full-on approach we could just do as we work with the plugins to add/fix, things. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
I would like to make the code base more readable so that it is easier to maintain and for first time contributors to feel more safe and self confident when proposing changes.
This list is up for discussion,
but these are some things slowing me down :
Object.assign
reduce
if else
without early return patternnested ternaries
These are very easy to write and near impossible to update when needed.
In general I avoid patterns that are hard to read or update.
A single ternary with scalar values is fine I think
But if it is nested or contains complex logic or function calls it becomes a bit of code that no one dares to touch.
Knowing which execution branches exist and when each path is triggered is also impossible with nested ternaries.
Object.assign
Object.assign
isn't that complex or hard to read, but it makes it less fluent to read the code and follow the data.It is mostly (ab)used for its ability to squash multiple objects into one.
Being explicit about which values are taken from which object makes it much more clear how data can flow.
reduce
No need to explain this one in too much detail.
The number of developers who truly understand
reduce
is just too limiting for code that should be accessible.Its main effect is as a gatekeeping tool even when not intended as such.
one line closures
This makes code harder to refactor and change.
Having each bit of logic on each own line is easier to read, change and track in git.
nested
if else
without early return patternThe likely hood of having unexpected execution paths is high.
These are sometimes also not bugs but appear as features.
Using early returns reduces the likelihood of this and overall results in a more well defined program.
unnecessary regex
Only rarely do we need a regex in these plugins.
If we are modifying CSS selectors or values we should use their respective parsers.
regex is not needed to do this and will only cause bugs.
If we want to detect something we can often use
indexOf
,includes
,...This will be less accurate but still good enough to create fast paths to skip execution of a transform.
':has(> img)'.includes(':has(')
false positive :
'[foo=":has(> img)"]'.includes(':has(')
The false positive is fine.
It is rare and the selector parser will handle it correctly.
Less regex makes it easier for contributors to change things they are not fully familiar with.
a11y is something I would also like to take into account going forward.
We already updated every file to have tabs which was a low effort change to make the code base more accessible.
Having consistent casing and good function/variable naming will also improve accessibility.
This can't be done in a single pass but I want to start this discussion so that it becomes something we take into account with each update.
Beta Was this translation helpful? Give feedback.
All reactions