Refactor/biome code style #628
Conversation
|
| @@ -144,6 +142,7 @@ export function applyOverride() { | |||
| const hookResolved = isApp() | |||
| ? requestMapApp.get(request) | |||
| : requestMapPage.get(request); | |||
| //! biome-ignore lint/style/noParameterAssign: <explanation> <-- doesn't work | |||
There was a problem hiding this comment.
It seems there's an issue with Biome. Here’s a reproduction link: Biome Playground
There was a problem hiding this comment.
Just set it to // biome-ignore lint: Should be lint/style/noParameterAssign, but there is a bug in biome
There was a problem hiding this comment.
+1.
And extra +1 if there is a link to the biome bug
There was a problem hiding this comment.
The issue was confirmed: biomejs/biome#4519
commit: |
|
Should this PR be rebased and merged? |
Definitely. One thing that we need to be extra careful with this PR is about every file with |
Ok, I'll rebase it today. |
maxmorozoff
force-pushed
the
refactor/biome-code-style
branch
from
082c1be
to
95eac7d
Compare
- Fixed linting issues related to the rule - Restored the rule to default settings
- Fixed linting issues related to the rule - Restored the rule to default settings
- Fixed linting issues related to the rule - Restored the rule to default settings
maxmorozoff
force-pushed
the
refactor/biome-code-style
branch
from
95eac7d
to
8751b03
Compare
|
The only remaining warning is related to a Biome issue. Should I refactor this part to resolve the warning by using a local variable, or would it be better to add a TODO comment to revisit this after the Biome issue is resolved? $ pnpm lint
> [email protected] lint /opennextjs-aws
> biome check
/opennextjs-aws/packages/open-next/src/core/require-hooks.ts:146:23 lint/style/noParameterAssign ━━━━━━━━━━
ℹ Reassigning a function parameter is confusing.
144 │ : requestMapPage.get(request);
145 │ //! biome-ignore lint/style/noParameterAssign: <explanation> <-- doesn't work
> 146 │ if (hookResolved) request = hookResolved;
│ ^^^^^^^
147 │ return originalResolveFilename.call(mod, request, parent, isMain, options);
148 │
ℹ The parameter is declared here:
135 │ requestMapApp: Map<string, string>,
136 │ requestMapPage: Map<string, string>,
> 137 │ request: string,
│ ^^^^^^^^^^^^^^^
138 │ parent: any,
139 │ isMain: boolean,
ℹ Use a local variable instead.
Checked 302 files in 121ms. No fixes applied. |
|
@maxmorozoff About the issue, why did you change it back to a normal function with a biome ignore #627 (comment) I'll do a proper review tomorrow |
…ns and ignore rule" This reverts commit b2a8203. Resolved here: opennextjs#627 (comment)
Didn't see your reply - my apologies. |
No description provided.