lint & fix #1808
lint & fix #1808
Conversation
| @@ -102,7 +102,6 @@ export async function onPaid ({ invoice, actIds }, { tx }) { | |||
| } | |||
| }) | |||
| acts = await tx.itemAct.findMany({ where: { invoiceId: invoice.id }, include: { item: true } }) | |||
| actIds = acts.map(act => act.id) | |||
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected], npm/[email protected] |
| @@ -16,7 +16,7 @@ export default function HiddenWalletSummary ({ abbreviate, fixedWidth }) { | |||
|
|
|||
| return ( | |||
| <span | |||
| className='text-monospace' style={{ whiteSpace: 'pre-wrap' }} align='right' onPointerEnter={() => setHover(true)} onPointerLeave={() => setHover(false)} | |||
| className='text-monospace' style={{ whiteSpace: 'pre-wrap', textAlign: 'right' }} onPointerEnter={() => setHover(true)} onPointerLeave={() => setHover(false)} | |||
There was a problem hiding this comment.
this is probably how it was intended, but this entire file looks like it is not being used anymore
| let variant = 'default' | ||
| let status = 'waiting for you' | ||
| let variant | ||
| let status |
There was a problem hiding this comment.
other defaults are set in the else block
| @@ -62,7 +62,7 @@ function ErrorImage ({ statusCode }) { | |||
| return <Image className='rounded-1 shadow-sm' width='500' height='376' src={`${process.env.NEXT_PUBLIC_ASSET_PREFIX}/falling.gif`} fluid /> | |||
| } | |||
|
|
|||
| export default function Error ({ statusCode }) { | |||
| export default function ErrorPage ({ statusCode }) { | |||
There was a problem hiding this comment.
was a potential global shadowing inside this file
| logger = walletLogger({ wallet, models }) | ||
| bolt11 = invoice | ||
| const logger = walletLogger({ wallet, models }) | ||
| const bolt11 = invoice |
There was a problem hiding this comment.
if the only thing the catch does is logging the error and rethrow, there is no point in wrapping all the rest of the code in the try-catch, also eslint didn't like this.
There was a problem hiding this comment.
This looks like it caught some stuff (largely inconsequential from the looks of it, but they'd confuse a newcomer, so nice to be rid of them).
We already have a linter (and we add a nextjs plugin) so I guess I'd be curious:
- which rules this is adding that we don't already have with the existing linters (the delta - do we know?)
- why we chose the rules that were chosen and not all the rules those plugins provide
- why we should implement these plugins into a separate linter rather than plugging into to the existing one
Maybe those things are obvious, but I'm not super familiar with lint rules and managing them.
I don't know much about standardjs, maybe it is possible to achieve the same goal in a different way, but the current setup seems to be targeted mostly at detecting formatting deviations, while the eslint config i've added here is mostly aimed at detecting oversights and bad patterns that make the code fragile or confusing.
The suggested rules for the plugins are too strict for sn (they were finding 300+ issues) so i've disabled those that I thought were considered undesired, and those that are just outright too strict to make sense in the codebase , just by my experience with the project (no scientific method here, and might need more tweaks). I've disabled also the rules related to improving regexes, that i think are not very important, and would need some special attention.
the rule set is here and here, minus the ones disabled. Ultimately my goal here was to add something to make the review process lighter by having some form of guidance to catch stylistic issues and avoid nitpicking in prs for new contributors (make the unwritten rules written), and to catch oversights from veterans. |
Proposal to add a permissive eslint config that catches some issues while also making the sndev coding style pass.
All deps are devDependencies. Usage is
npm run eslint.I fixed all the issues unrelated to coding style, that were just dead code, unused imports, wrong react pops and a potential global shadowing that did not cause any issue yet.
It would probably be useful to add graphql-eslint plugin, but i couldn't figure out how to provide the schema that it needs.