Skip to content
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.

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

chore: use Biome as linter #111

Merged
merged 9 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 7 additions & 26 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,21 @@ const { builtinModules } = require('module');

module.exports = {
extends: [
'plugin:@typescript-eslint/recommended-type-checked',
'plugin:@typescript-eslint/stylistic-type-checked',
'prettier',
'plugin:@typescript-eslint/recommended-type-checked'
],
parser: '@typescript-eslint/parser',
parserOptions: {
project: ['./packages/*/tsconfig.json', './tsconfig.eslint.json'],
tsconfigRootDir: __dirname,
},
plugins: ['@typescript-eslint', 'prettier', 'no-only-tests'],
plugins: ['@typescript-eslint', 'no-only-tests'],
rules: {
// These off/configured-differently-by-default rules fit well for us
'@typescript-eslint/array-type': ['error', { default: 'array-simple' }],
'@typescript-eslint/no-unused-vars': [
'error',
{ argsIgnorePattern: '^_', ignoreRestSiblings: true },
],
'no-only-tests/no-only-tests': 'error',
'@typescript-eslint/no-shadow': ['error'],
'no-console': 'warn',

'@typescript-eslint/no-shadow': "off",
'no-console': 'off',
'@typescript-eslint/no-unused-vars': "off",
"@typescript-eslint/ban-types": "off",
// Todo: do we want these?
'@typescript-eslint/array-type': 'off',
'@typescript-eslint/ban-ts-comment': 'off',
Expand Down Expand Up @@ -80,19 +74,6 @@ module.exports = {
rules: {
'no-console': 'off',
},
},
{
files: ['packages/integrations/**/*.ts'],
rules: {
'no-console': ['error', { allow: ['warn', 'error', 'info', 'debug'] }],
},
},
{
files: ['benchmark/**/*.js'],
rules: {
'@typescript-eslint/no-unused-vars': 'off',
'no-console': 'off',
},
},
}
],
};
8 changes: 7 additions & 1 deletion biome.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@
"enabled": true
},
"linter": {
"enabled": false
"enabled": true,
"rules": {
"suspicious": {
"noConsoleLog": "off",
Copy link
Member

Choose a reason for hiding this comment

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

Can we still get a warn?

Copy link
Member Author

Choose a reason for hiding this comment

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

You would get a bunch of warnings, which are annoying I felt annoying. Ideally, we could use an override, but they are currently buggy. Are you still happy with a warning?

Copy link
Member

Choose a reason for hiding this comment

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

Hm that is a tricky one. In general we don't really want to use console.log() for anything user-facing, everything user-facing should probably use astro's logger or error. So getting a warning, sounds feasible. However I was also annoyed by it before, so disabling this makes also sense.

I'm happy both ways, so maybe keep it off for now!

"noExplicitAny": "off"
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason to disable this rule?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are a bunch of any that I didn't want to touch, so I turned off the rule. How do you want to proceed?

Copy link
Member

Choose a reason for hiding this comment

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

Oh I didn't know we had them :/
Let's keep the rule off for now, and I'll double check the code, to get rid of those anys

}
}
}
}
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
"build": "turbo run build --filter=\"@astrojs/*\"",
"build:ci": "turbo run build:ci --filter=\"@astrojs/*\"",
"format:ci": "pnpm run format",
"format": "biome check --apply ./ && prettier -w \"**/*\" --ignore-unknown --cache",
"format": "biome check --linter-enabled=false --apply ./ && prettier -w \"**/*\" --ignore-unknown --cache",
alexanderniebuhr marked this conversation as resolved.
Show resolved Hide resolved
"test": "turbo run test --concurrency=1 --filter=astro --filter=create-astro --filter=\"@astrojs/*\"",
"benchmark": "astro-benchmark",
"lint": "eslint . --report-unused-disable-directives",
"lint": "biome lint ./ && eslint . --report-unused-disable-directives",
"lint:fix": "biome lint --apply ./",
"version": "changeset version && pnpm install --no-frozen-lockfile && pnpm run format",
"preinstall": "npx only-allow pnpm"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/cloudflare/src/entrypoints/server.advanced.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function createExports(manifest: SSRManifest) {
return env.ASSETS.fetch(request);
}

let routeData = app.match(request);
const routeData = app.match(request);
Reflect.set(
request,
Symbol.for('astro.clientAddress'),
Expand All @@ -57,7 +57,7 @@ export function createExports(manifest: SSRManifest) {
},
};

let response = await app.render(request, routeData, locals);
const response = await app.render(request, routeData, locals);

if (app.setCookieHeaders) {
for (const setCookieHeader of app.setCookieHeaders(response)) {
Expand Down
4 changes: 2 additions & 2 deletions packages/cloudflare/src/entrypoints/server.directory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function createExports(manifest: SSRManifest) {
return env.ASSETS.fetch(request);
}

let routeData = app.match(request);
const routeData = app.match(request);
Reflect.set(
request,
Symbol.for('astro.clientAddress'),
Expand All @@ -50,7 +50,7 @@ export function createExports(manifest: SSRManifest) {
},
};

let response = await app.render(request, routeData, locals);
const response = await app.render(request, routeData, locals);

if (app.setCookieHeaders) {
for (const setCookieHeader of app.setCookieHeaders(response)) {
Expand Down
39 changes: 17 additions & 22 deletions packages/cloudflare/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ export default function createIntegration(args?: Options): AstroIntegration {
});
}

const outputFiles: Array<string> = await glob(`**/*`, {
const outputFiles: Array<string> = await glob('**/*', {
cwd: outputDir,
filesOnly: true,
});
Expand Down Expand Up @@ -410,20 +410,16 @@ export default function createIntegration(args?: Options): AstroIntegration {
.map((route) => {
if (route.component === 'src/pages/404.astro' && route.prerender === false)
notFoundIsSSR = true;
const includePattern =
'/' +
route.segments
.flat()
.map((segment) => (segment.dynamic ? '*' : segment.content))
.join('/');
const includePattern = `/${route.segments
.flat()
.map((segment) => (segment.dynamic ? '*' : segment.content))
.join('/')}`;

const regexp = new RegExp(
'^\\/' +
route.segments
.flat()
.map((segment) => (segment.dynamic ? '(.*)' : segment.content))
.join('\\/') +
'$'
`^\\/${route.segments
.flat()
.map((segment) => (segment.dynamic ? '(.*)' : segment.content))
.join('\\/')}$`
);

return {
Expand All @@ -442,7 +438,7 @@ export default function createIntegration(args?: Options): AstroIntegration {
.filter((file: string) => cloudflareSpecialFiles.indexOf(file) < 0)
.map((file: string) => `/${file.replace(/\\/g, '/')}`);

for (let page of pages) {
for (const page of pages) {
let pagePath = prependForwardSlash(page.pathname);
if (_config.base !== '/') {
const base = _config.base.endsWith('/') ? _config.base.slice(0, -1) : _config.base;
Expand All @@ -467,15 +463,14 @@ export default function createIntegration(args?: Options): AstroIntegration {
const parts = line.split(' ');
if (parts.length < 2) {
return null;
} else {
// convert /products/:id to /products/*
return (
parts[0]
.replace(/\/:.*?(?=\/|$)/g, '/*')
// remove query params as they are not supported by cloudflare
.replace(/\?.*$/, '')
);
}
// convert /products/:id to /products/*
return (
parts[0]
.replace(/\/:.*?(?=\/|$)/g, '/*')
// remove query params as they are not supported by cloudflare
.replace(/\?.*$/, '')
);
})
.filter(
(line, index, arr) => line !== null && arr.indexOf(line) === index
Expand Down
5 changes: 1 addition & 4 deletions packages/cloudflare/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ export function getProcessEnvProxy() {
get: (target, prop) => {
console.warn(
// NOTE: \0 prevents Vite replacement
`Unable to access \`import.meta\0.env.${prop.toString()}\` on initialization ` +
`as the Cloudflare platform only provides the environment variables per request. ` +
`Please move the environment variable access inside a function ` +
`that's only called after a request has been received.`
`Unable to access \`import.meta\0.env.${prop.toString()}\` on initialization as the Cloudflare platform only provides the environment variables per request. Please move the environment variable access inside a function that's only called after a request has been received.`
);
},
}
Expand Down
24 changes: 15 additions & 9 deletions packages/cloudflare/src/utils/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ export function isRemotePath(src: string) {
export function matchHostname(url: URL, hostname?: string, allowWildcard?: boolean) {
if (!hostname) {
return true;
} else if (!allowWildcard || !hostname.startsWith('*')) {
}
if (!allowWildcard || !hostname.startsWith('*')) {
return hostname === url.hostname;
} else if (hostname.startsWith('**.')) {
}
if (hostname.startsWith('**.')) {
const slicedHostname = hostname.slice(2); // ** length
return slicedHostname !== url.hostname && url.hostname.endsWith(slicedHostname);
} else if (hostname.startsWith('*.')) {
}
if (hostname.startsWith('*.')) {
const slicedHostname = hostname.slice(1); // * length
const additionalSubdomains = url.hostname
.replace(slicedHostname, '')
Expand All @@ -34,12 +37,15 @@ export function matchProtocol(url: URL, protocol?: string) {
export function matchPathname(url: URL, pathname?: string, allowWildcard?: boolean) {
if (!pathname) {
return true;
} else if (!allowWildcard || !pathname.endsWith('*')) {
}
if (!allowWildcard || !pathname.endsWith('*')) {
return pathname === url.pathname;
} else if (pathname.endsWith('/**')) {
}
if (pathname.endsWith('/**')) {
const slicedPathname = pathname.slice(0, -2); // ** length
return slicedPathname !== url.pathname && url.pathname.startsWith(slicedPathname);
} else if (pathname.endsWith('/*')) {
}
if (pathname.endsWith('/*')) {
const slicedPathname = pathname.slice(0, -1); // * length
const additionalPathChunks = url.pathname
.replace(slicedPathname, '')
Expand Down Expand Up @@ -91,11 +97,11 @@ export function joinPaths(...paths: (string | undefined)[]) {
.map((path, i) => {
if (i === 0) {
return removeTrailingForwardSlash(path);
} else if (i === paths.length - 1) {
}
if (i === paths.length - 1) {
return removeLeadingForwardSlash(path);
} else {
return trimSlashes(path);
}
return trimSlashes(path);
})
.join('/');
}
12 changes: 7 additions & 5 deletions packages/cloudflare/src/utils/local-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ class LocalRuntime {
this._astroConfig = astroConfig;
this._logger = logger;

let varBindings: Required<Pick<WorkerOptions, 'bindings'>>['bindings'] = {};
let kvBindings: Required<Pick<WorkerOptions, 'kvNamespaces'>>['kvNamespaces'] = [];
let d1Bindings: Required<Pick<WorkerOptions, 'd1Databases'>>['d1Databases'] = [];
let r2Bindings: Required<Pick<WorkerOptions, 'r2Buckets'>>['r2Buckets'] = [];
let durableObjectBindings: Required<Pick<WorkerOptions, 'durableObjects'>>['durableObjects'] =
const varBindings: Required<Pick<WorkerOptions, 'bindings'>>['bindings'] = {};
const kvBindings: Required<Pick<WorkerOptions, 'kvNamespaces'>>['kvNamespaces'] = [];
const d1Bindings: Required<Pick<WorkerOptions, 'd1Databases'>>['d1Databases'] = [];
const r2Bindings: Required<Pick<WorkerOptions, 'r2Buckets'>>['r2Buckets'] = [];
const durableObjectBindings: Required<Pick<WorkerOptions, 'durableObjects'>>['durableObjects'] =
{};

for (const bindingName in runtimeConfig.bindings) {
Expand Down Expand Up @@ -280,6 +280,8 @@ export class LocalWorkersRuntime extends LocalRuntime {
}

export class LocalPagesRuntime extends LocalRuntime {

// biome-ignore lint/complexity/noUselessConstructor: not types information yet, so we need to disable the rule for the time being
public constructor(
astroConfig: AstroConfig,
runtimeConfig: Extract<RUNTIME, { type: 'pages' }>,
Expand Down
14 changes: 7 additions & 7 deletions packages/cloudflare/src/utils/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ function findWranglerToml(
): string | undefined {
if (preferJson) {
return (
findUpSync(`wrangler.json`, { cwd: referencePath }) ??
findUpSync(`wrangler.toml`, { cwd: referencePath })
findUpSync('wrangler.json', { cwd: referencePath }) ??
findUpSync('wrangler.toml', { cwd: referencePath })
);
}
return findUpSync(`wrangler.toml`, { cwd: referencePath });
return findUpSync('wrangler.toml', { cwd: referencePath });
}
type File = {
file?: string;
Expand Down Expand Up @@ -117,9 +117,8 @@ function getVarsForDev(config: any, configPath: string | undefined): any {
...config.vars,
...loaded.parsed,
};
} else {
return config.vars;
}
return config.vars;
}

function parseConfig() {
Expand Down Expand Up @@ -179,12 +178,13 @@ export function getDOBindings(): Record<
> {
const { rawConfig } = parseConfig();
if (!rawConfig) return {};
if (!rawConfig?.durable_objects) return {};
if (!rawConfig.durable_objects) return {};
const output = new Object({}) as Record<
string,
{ scriptName?: string | undefined; unsafeUniqueKey?: string | undefined; className: string }
>;
for (const binding of rawConfig?.durable_objects.bindings) {
const bindings = rawConfig.durable_objects.bindings;
for (const binding of bindings) {
Reflect.set(output, binding.name, { className: binding.class_name });
}
return output;
Expand Down
2 changes: 1 addition & 1 deletion packages/cloudflare/src/utils/prependForwardSlash.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export function prependForwardSlash(path: string) {
return path[0] === '/' ? path : '/' + path;
return path[0] === '/' ? path : `/${path}`;
}
43 changes: 21 additions & 22 deletions packages/cloudflare/src/utils/wasm-module-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,33 +57,32 @@ export default wasmModule
if (isDev) {
// no need to wire up the assets in dev mode, just rewrite
return base64Module;
} else {
// just some shared ID
let hash = hashString(base64);
// emit the wasm binary as an asset file, to be picked up later by the esbuild bundle for the worker.
// give it a shared deterministic name to make things easy for esbuild to switch on later
const assetName = path.basename(filePath).split('.')[0] + '.' + hash + '.wasm';
this.emitFile({
type: 'asset',
// put it explicitly in the _astro assets directory with `fileName` rather than `name` so that
// vite doesn't give it a random id in its name. We need to be able to easily rewrite from
// the .mjs loader and the actual wasm asset later in the ESbuild for the worker
fileName: path.join(assetsDirectory, assetName),
source: fs.readFileSync(filePath),
});
}
// just some shared ID
const hash = hashString(base64);
// emit the wasm binary as an asset file, to be picked up later by the esbuild bundle for the worker.
// give it a shared deterministic name to make things easy for esbuild to switch on later
const assetName = `${path.basename(filePath).split('.')[0]}.${hash}.wasm`;
this.emitFile({
type: 'asset',
// put it explicitly in the _astro assets directory with `fileName` rather than `name` so that
// vite doesn't give it a random id in its name. We need to be able to easily rewrite from
// the .mjs loader and the actual wasm asset later in the ESbuild for the worker
fileName: path.join(assetsDirectory, assetName),
source: fs.readFileSync(filePath),
});

// however, by default, the SSG generator cannot import the .wasm as a module, so embed as a base64 string
const chunkId = this.emitFile({
type: 'prebuilt-chunk',
fileName: assetName + '.mjs',
code: base64Module,
});
// however, by default, the SSG generator cannot import the .wasm as a module, so embed as a base64 string
const chunkId = this.emitFile({
type: 'prebuilt-chunk',
fileName: `${assetName}.mjs`,
code: base64Module,
});

return `
return `
import wasmModule from "__WASM_ASSET__${chunkId}.wasm.mjs";
export default wasmModule;
`;
}
},

// output original wasm file relative to the chunk
Expand Down