Skip to content

fix all the remaining eslint warnings #545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 13, 2024
Merged

fix all the remaining eslint warnings #545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .eslintrc.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = {
],
"sonarjs/elseif-without-else": "warn",
"sonarjs/no-duplicate-string": "warn",
"sonarjs/cognitive-complexity": "warn",
"sonarjs/cognitive-complexity": ["warn", 35],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

35 allows most functions in the codebase, I could just add disabling comment to each one instead if preferred (there were around 10-ish or something I think)

I did not attempt on splitting the functions as I am not really familiar with the codebase here (but I am happy to give it a go if you want 🙂)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.
I need to take a closer look at the PR later today, I remember that there was a reason that it was this order. Might be old code


// We add some typescript rules - The recommended rules breaks too much stuff
// TODO: We should add more rules, especially around typescript types
Expand All @@ -29,8 +29,6 @@ module.exports = {
],

"@typescript-eslint/unbound-method": "error",

"@typescript-eslint/no-non-null-assertion": "warn",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am personally not a fan of this one 😅

if someone uses a non-null assertion I think that's generally for a good reason so having to add an eslint disabling comment for it feels very unnecessary/annoying to me 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm fine with this
We just need to verify on every PR to make sure it's not used unwisely

},
overrides: [
{
Expand Down
159 changes: 87 additions & 72 deletions packages/open-next/src/adapters/cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,80 +284,95 @@ export default class S3Cache {
.getStore()
?.pendingPromiseRunner.withResolvers<void>();
try {
if (data?.kind === "ROUTE" || data?.kind === "APP_ROUTE") {
const { body, status, headers } = data;
await globalThis.incrementalCache.set(
key,
{
type: "route",
body: body.toString(
isBinaryContentType(String(headers["content-type"]))
? "base64"
: "utf8",
),
meta: {
status,
headers,
},
},
false,
);
} else if (data?.kind === "PAGE" || data?.kind === "PAGES") {
const { html, pageData, status, headers } = data;
const isAppPath = typeof pageData === "string";
if (isAppPath) {
await globalThis.incrementalCache.set(
key,
{
type: "app",
html,
rsc: pageData,
meta: {
status,
headers,
if (data === null || data === undefined) {
await globalThis.incrementalCache.delete(key);
} else {
switch (data.kind) {
case "ROUTE":
case "APP_ROUTE":
const { body, status, headers } = data;
await globalThis.incrementalCache.set(
key,
{
type: "route",
body: body.toString(
isBinaryContentType(String(headers["content-type"]))
? "base64"
: "utf8",
),
meta: {
status,
headers,
},
},
},
false,
);
} else {
await globalThis.incrementalCache.set(
key,
{
type: "page",
html,
json: pageData,
},
false,
);
false,
);
break;
case "PAGE":
case "PAGES": {
const { html, pageData, status, headers } = data;
const isAppPath = typeof pageData === "string";
if (isAppPath) {
await globalThis.incrementalCache.set(
key,
{
type: "app",
html,
rsc: pageData,
meta: {
status,
headers,
},
},
false,
);
} else {
await globalThis.incrementalCache.set(
key,
{
type: "page",
html,
json: pageData,
},
false,
);
}
break;
}
case "APP_PAGE": {
const { html, rscData, headers, status } = data;
await globalThis.incrementalCache.set(
key,
{
type: "app",
html,
rsc: rscData.toString("utf8"),
meta: {
status,
headers,
},
},
false,
);
break;
}
case "FETCH":
await globalThis.incrementalCache.set<true>(key, data, true);
break;
case "REDIRECT":
await globalThis.incrementalCache.set(
key,
{
type: "redirect",
props: data.props,
},
false,
);
break;
case "IMAGE":
// Not implemented
break;
}
} else if (data?.kind === "APP_PAGE") {
const { html, rscData, headers, status } = data;
await globalThis.incrementalCache.set(
key,
{
type: "app",
html,
rsc: rscData.toString("utf8"),
meta: {
status,
headers,
},
},
false,
);
} else if (data?.kind === "FETCH") {
await globalThis.incrementalCache.set<true>(key, data, true);
} else if (data?.kind === "REDIRECT") {
await globalThis.incrementalCache.set(
key,
{
type: "redirect",
props: data.props,
},
false,
);
} else if (data === null || data === undefined) {
await globalThis.incrementalCache.delete(key);
}
// Write derivedTags to dynamodb
// If we use an in house version of getDerivedTags in build we should use it here instead of next's one
Expand Down
1 change: 1 addition & 0 deletions packages/open-next/src/adapters/plugins/without-routing/requestHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable unused-imports/no-unused-vars */
import { InternalEvent } from "types/open-next";

import { MiddlewareOutputEvent } from "../../../core/routingHandler";
Expand Down
1 change: 1 addition & 0 deletions packages/open-next/src/build/copyTracedFiles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ function copyPatchFile(outputDir: string) {
copyFileSync(patchFile, outputPatchFile);
}

// eslint-disable-next-line sonarjs/cognitive-complexity
export async function copyTracedFiles(
buildOutputPath: string,
packagePath: string,
Expand Down
17 changes: 10 additions & 7 deletions packages/open-next/src/build/generateOutput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ interface OpenNextOutput {
};
}

const indexHandler = "index.hander";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a typo here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry 😓

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry 😓

Typos happens to everyone. Your PR made the code more DRY and easier to read. Great work ⭐️


async function canStream(opts: FunctionOptions) {
if (!opts.override?.wrapper) {
return false;
Expand Down Expand Up @@ -161,6 +163,7 @@ function prefixPattern(basePath: string) {
};
}

// eslint-disable-next-line sonarjs/cognitive-complexity
export async function generateOutput(
outputPath: string,
config: OpenNextConfig,
Expand All @@ -183,7 +186,7 @@ export async function generateOutput(
if (value.placement === "global") {
edgeFunctions[key] = {
bundle: `.open-next/functions/${key}`,
handler: "index.handler",
handler: indexHandler,
...(await extractOverrideFn(value.override)),
};
}
Expand Down Expand Up @@ -228,7 +231,7 @@ export async function generateOutput(
},
imageOptimizer: {
type: "function",
handler: "index.handler",
handler: indexHandler,
bundle: ".open-next/image-optimization-function",
streaming: false,
imageLoader: await extractOverrideName(
Expand All @@ -247,7 +250,7 @@ export async function generateOutput(
}
: {
type: "function",
handler: "index.handler",
handler: indexHandler,
bundle: ".open-next/server-functions/default",
streaming: defaultOriginCanstream,
...(await extractOverrideFn(config.default.override)),
Expand All @@ -274,7 +277,7 @@ export async function generateOutput(
const streaming = await canStream(value);
origins[key] = {
type: "function",
handler: "index.handler",
handler: indexHandler,
bundle: `.open-next/server-functions/${key}`,
streaming,
...(await extractOverrideFn(value.override)),
Expand Down Expand Up @@ -348,19 +351,19 @@ export async function generateOutput(
disableIncrementalCache: config.dangerous?.disableIncrementalCache,
disableTagCache: config.dangerous?.disableTagCache,
warmer: {
handler: "index.handler",
handler: indexHandler,
bundle: ".open-next/warmer-function",
},
initializationFunction: isTagCacheDisabled
? undefined
: {
handler: "index.handler",
handler: indexHandler,
bundle: ".open-next/dynamodb-provider",
},
revalidationFunction: config.dangerous?.disableIncrementalCache
? undefined
: {
handler: "index.handler",
handler: indexHandler,
bundle: ".open-next/revalidation-function",
},
},
Expand Down
1 change: 1 addition & 0 deletions packages/open-next/src/core/routing/i18n/accept-header.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface Options {
type: "accept-language";
}

// eslint-disable-next-line sonarjs/cognitive-complexity
function parse(
raw: string,
preferences: string[] | undefined,
Expand Down
2 changes: 1 addition & 1 deletion packages/open-next/src/core/routingHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export interface MiddlewareOutputEvent {

// Add the locale prefix to the regex so we correctly match the rawPath
const optionalLocalePrefixRegex = !!RoutesManifest.locales.length
? `^/(?:${RoutesManifest.locales.map((locale) => `${locale}/?`).join("|")})?`
? `^/(?:${RoutesManifest.locales.map((locale) => locale + "/?").join("|")})?`
: "^/";

// Add the basepath prefix to the regex so we correctly match the rawPath
Expand Down
1 change: 1 addition & 0 deletions packages/open-next/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ function parseArgs() {
acc[key] = undefined;
} else if (self[ind + 1]) {
acc[key] = self[ind + 1];
// eslint-disable-next-line sonarjs/elseif-without-else
} else if (!self[ind + 1]) {
acc[key] = undefined;
}
Expand Down
1 change: 1 addition & 0 deletions packages/open-next/src/minimize-js.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable */
// Copied and modified from node-minify-all-js by Adones Pitogo
// https://github.com/adonespitogo/node-minify-all-js/blob/master/index.js

Expand Down
2 changes: 1 addition & 1 deletion packages/open-next/src/plugins/edge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export function openNextEdgePlugins({
});
}

build.onResolve({ filter: /\.(mjs|wasm)$/g }, (args) => {
build.onResolve({ filter: /\.(mjs|wasm)$/g }, () => {
return {
external: true,
};
Expand Down
Loading