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

feat: use typescript-eslint@v6's reworked configs #7425

Merged
merged 14 commits into from
Jul 3, 2023
Merged
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
**/*.d.ts
packages/**/*.min.js
packages/**/dist/**/*
packages/**/fixtures/**/*
packages/webapi/**/*
Expand Down
55 changes: 43 additions & 12 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
@@ -1,23 +1,53 @@
module.exports = {
extends: [
Copy link
Member

Choose a reason for hiding this comment

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

I wonder, since this is a breaking change, should we try to move to use the new eslint configuration format? https://eslint.org/docs/latest/use/configure/configuration-files-new

'plugin:@typescript-eslint/recommended-type-checked',
'plugin:@typescript-eslint/stylistic-type-checked',
'prettier',
],
parser: '@typescript-eslint/parser',
extends: ['plugin:@typescript-eslint/recommended', 'prettier'],
parserOptions: {
project: ['./packages/*/tsconfig.json', './tsconfig.eslint.json'],
tsconfigRootDir: __dirname,
},
plugins: ['@typescript-eslint', 'prettier', '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', { ignoreRestSiblings: true }],
'no-only-tests/no-only-tests': 'error',
'@typescript-eslint/no-shadow': ['error'],
'no-console': 'warn',

// Todo: do we want these?
'@typescript-eslint/array-type': 'off',
'@typescript-eslint/ban-ts-comment': 'off',
'@typescript-eslint/camelcase': 'off',
'@typescript-eslint/explicit-module-boundary-types': 'off',
'@typescript-eslint/class-literal-property-style': 'off',
'@typescript-eslint/consistent-indexed-object-style': 'off',
'@typescript-eslint/consistent-type-definitions': 'off',
'@typescript-eslint/dot-notation': 'off',
'@typescript-eslint/no-base-to-string': 'off',
'@typescript-eslint/no-confusing-void-expression': 'off',
'@typescript-eslint/no-empty-function': 'off',
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
'@typescript-eslint/no-unused-vars': 'off',
'@typescript-eslint/no-use-before-define': 'off',
'@typescript-eslint/no-var-requires': 'off',
'@typescript-eslint/no-floating-promises': 'off',
'@typescript-eslint/no-misused-promises': 'off',
'@typescript-eslint/no-redundant-type-constituents': 'off',
'@typescript-eslint/no-this-alias': 'off',
'no-console': 'warn',
'@typescript-eslint/no-unsafe-argument': 'off',
'@typescript-eslint/no-unsafe-assignment': 'off',
'@typescript-eslint/no-unsafe-call': 'off',
'@typescript-eslint/no-unsafe-member-access': 'off',
'@typescript-eslint/no-unsafe-return': 'off',
'@typescript-eslint/prefer-nullish-coalescing': 'off',
'@typescript-eslint/prefer-string-starts-ends-with': 'off',
'@typescript-eslint/require-await': 'off',
'@typescript-eslint/restrict-plus-operands': 'off',
'@typescript-eslint/restrict-template-expressions': 'off',
'@typescript-eslint/sort-type-constituents': 'off',
'@typescript-eslint/unbound-method': 'off',
bluwy marked this conversation as resolved.
Show resolved Hide resolved

// These rules enabled by the preset configs don't work well for us
'@typescript-eslint/await-thenable': 'off',
'prefer-const': 'off',
'no-shadow': 'off',
'@typescript-eslint/no-shadow': ['error'],
'no-only-tests/no-only-tests': 'error',
},
overrides: [
{
Expand All @@ -41,6 +71,7 @@ module.exports = {
{
files: ['benchmark/**/*.js'],
rules: {
'@typescript-eslint/no-unused-vars': 'off',
'no-console': 'off',
},
},
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ env:
jobs:
lint:
name: Lint
timeout-minutes: 3
timeout-minutes: 5
runs-on: ubuntu-latest
steps:
- name: Check out repository
Expand All @@ -50,6 +50,9 @@ jobs:
- name: Install dependencies
run: pnpm install

- name: Build (ignoring failures)
run: pnpm run build || true
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

# Lint autofix cannot run on forks, so just skip those! See https://github.com/wearerequired/lint-action/issues/13
- name: Lint (External)
if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.owner.login != github.repository_owner }}
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"test:e2e": "cd packages/astro && pnpm playwright install && pnpm run test:e2e",
"test:e2e:match": "cd packages/astro && pnpm playwright install && pnpm run test:e2e:match",
"benchmark": "astro-benchmark",
"lint": "eslint --cache .",
"lint": "eslint . --report-unused-disable-directives",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not strictly necessary, but I've found this to be a nice cleanup.

Copy link
Member

Choose a reason for hiding this comment

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

I think we'd want to still keep the --cache too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ehh I've found the cache to be ... unreliable when working with type checking. Theoretically a file's lint result could be impacted by a couple of other factors -other files it imports types from, TSConfig settings- so caching becomes difficult.

Copy link
Member

Choose a reason for hiding this comment

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

I won't deny that I've seen --cache acting strange before, so I could be persuaded removing it for now if there's no rejections from others.

"version": "changeset version && node ./scripts/deps/update-example-versions.js && pnpm install --no-frozen-lockfile && pnpm run format",
"preinstall": "npx only-allow pnpm"
},
Expand Down Expand Up @@ -78,8 +78,8 @@
"@changesets/changelog-github": "^0.4.8",
"@changesets/cli": "^2.26.1",
"@types/node": "^18.16.18",
"@typescript-eslint/eslint-plugin": "^5.60.0",
"@typescript-eslint/parser": "^5.60.0",
"@typescript-eslint/eslint-plugin": "6.0.0-alpha.158",
"@typescript-eslint/parser": "6.0.0-alpha.158",
"esbuild": "^0.17.19",
"eslint": "^8.43.0",
"eslint-config-prettier": "^8.8.0",
Expand Down
1 change: 0 additions & 1 deletion packages/astro/astro.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#!/usr/bin/env node
/* eslint-disable no-console */
'use strict';

// ISOMORPHIC FILE: NO TOP-LEVEL IMPORT/REQUIRE() ALLOWED
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/e2e/css-sourcemaps.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from '@playwright/test';
import { getColor, isWindows, testFactory } from './test-utils.js';
import { isWindows, testFactory } from './test-utils.js';

const test = testFactory({
root: './fixtures/css/',
Expand Down
2 changes: 0 additions & 2 deletions packages/astro/e2e/hydration-race.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ test.describe('Hydration race', () => {
test('Islands inside of slots hydrate', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/slot'));

const html = await page.content();

const one = page.locator('#one');
await expect(one, 'updated text').toHaveText('Hello One in the client');

Expand Down
3 changes: 1 addition & 2 deletions packages/astro/e2e/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ for (let i = 0; i < testFiles.length; i++) {
}

export function loadFixture(inlineConfig) {
if (!inlineConfig || !inlineConfig.root)
throw new Error("Must provide { root: './fixtures/...' }");
if (!inlineConfig?.root) throw new Error("Must provide { root: './fixtures/...' }");

// resolve the relative root (i.e. "./fixtures/tailwindcss") to a full filepath
// without this, the main `loadFixture` helper will resolve relative to `packages/astro/test`
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/assets/image-endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const get: APIRoute = async ({ request }) => {
const url = new URL(request.url);
const transform = await imageService.parseURL(url, imageServiceConfig);

if (!transform || !transform.src) {
if (!transform?.src) {
throw new Error('Incorrect transform returned by `parseURL`');
}

Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/assets/services/squoosh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const service: LocalImageService = {
async transform(inputBuffer, transformOptions) {
const transform: BaseServiceTransform = transformOptions as BaseServiceTransform;

let format = transform.format!;
let format = transform.format;

const operations: Operation[] = [];

Expand Down
2 changes: 0 additions & 2 deletions packages/astro/src/assets/services/vendor/squoosh/codecs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,6 @@ export const codecs = {
avif: {
name: 'AVIF',
extension: 'avif',
// eslint-disable-next-line no-control-regex
detectors: [/^\x00\x00\x00 ftypavif\x00\x00\x00\x00/],
dec: () =>
instantiateEmscriptenWasm(avifDec as DecodeModuleFactory, avifDecWasm),
Expand Down Expand Up @@ -322,7 +321,6 @@ export const codecs = {
oxipng: {
name: 'OxiPNG',
extension: 'png',
// eslint-disable-next-line no-control-regex
detectors: [/^\x89PNG\x0D\x0A\x1A\x0A/],
dec: async () => {
await pngEncDecInit()
Expand Down
8 changes: 4 additions & 4 deletions packages/astro/src/assets/services/vendor/squoosh/image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ export async function processBuffer(

switch (encoding) {
case 'avif':
return await impl.encodeAvif(imageData, { quality }) as Uint8Array;
return await impl.encodeAvif(imageData, { quality });
case 'jpeg':
case 'jpg':
return await impl.encodeJpeg(imageData, { quality }) as Uint8Array;
return await impl.encodeJpeg(imageData, { quality });
case 'png':
return await impl.encodePng(imageData) as Uint8Array;
return await impl.encodePng(imageData);
case 'webp':
return await impl.encodeWebp(imageData, { quality }) as Uint8Array;
return await impl.encodeWebp(imageData, { quality });
default:
throw Error(`Unsupported encoding format`)
}
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/assets/vendor/image-size/types/jp2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export const JP2: IImage = {
switch (nextBoxType) {
case BoxTypes.rreq:
// WHAT ARE THESE 4 BYTES?????
// eslint-disable-next-line no-case-declarations
const MAGIC = 4
offset = offset + 4 + MAGIC + calculateRREQLength(buffer.slice(offset + 4))
return parseIHDR(buffer.slice(offset + 8, offset + 24))
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/assets/vendor/image-size/types/pnm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const handlers: { [type: string]: Handler} = {
let dimensions: string[] = []

while (lines.length > 0) {
const line = lines.shift() as string
const line = lines.shift()!
ematipico marked this conversation as resolved.
Show resolved Hide resolved
if (line[0] === '#') {
continue
}
Expand All @@ -39,7 +39,7 @@ const handlers: { [type: string]: Handler} = {
pam: (lines) => {
const size: { [key: string]: number } = {}
while (lines.length > 0) {
const line = lines.shift() as string
const line = lines.shift()!
if (line.length > 16 || line.charCodeAt(0) > 128) {
continue
}
Expand Down
20 changes: 10 additions & 10 deletions packages/astro/src/assets/vendor/image-size/types/svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ function parseLength(len: string) {
function parseViewbox(viewbox: string): IAttributes {
const bounds = viewbox.split(' ')
return {
height: parseLength(bounds[3]) as number,
width: parseLength(bounds[2]) as number
height: parseLength(bounds[3])!,
width: parseLength(bounds[2])!
}
}

Expand All @@ -51,21 +51,21 @@ function parseAttributes(root: string): IAttributes {
const height = root.match(extractorRegExps.height)
const viewbox = root.match(extractorRegExps.viewbox)
return {
height: height && parseLength(height[2]) as number,
viewbox: viewbox && parseViewbox(viewbox[2]) as IAttributes,
width: width && parseLength(width[2]) as number,
height: height && parseLength(height[2])!,
viewbox: viewbox && parseViewbox(viewbox[2])!,
width: width && parseLength(width[2])!,
}
}

function calculateByDimensions(attrs: IAttributes): ISize {
return {
height: attrs.height as number,
width: attrs.width as number,
height: attrs.height!,
width: attrs.width!,
}
}

function calculateByViewbox(attrs: IAttributes, viewbox: IAttributes): ISize {
const ratio = (viewbox.width as number) / (viewbox.height as number)
const ratio = (viewbox.width!) / (viewbox.height!)
if (attrs.width) {
return {
height: Math.floor(attrs.width / ratio),
Expand All @@ -79,8 +79,8 @@ function calculateByViewbox(attrs: IAttributes, viewbox: IAttributes): ISize {
}
}
return {
height: viewbox.height as number,
width: viewbox.width as number,
height: viewbox.height!,
width: viewbox.width!,
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/assets/vendor/image-size/types/tiff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function extractTags(buffer: Buffer, isBigEndian: boolean) {
const tags: {[key: number]: number} = {}

let temp: Buffer | undefined = buffer
while (temp && temp.length) {
while (temp?.length) {
const code = readUInt(temp, 16, 0, isBigEndian)
const type = readUInt(temp, 16, 2, isBigEndian)
const length = readUInt(temp, 32, 4, isBigEndian)
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/cli/check/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable no-console */
import {
AstroCheck,
DiagnosticSeverity,
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/content/error-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const errorMap: ZodErrorMap = (baseError, ctx) => {
// raise a single error when `key` does not match:
// > Did not match union.
// > key: Expected `'tutorial' | 'blog'`, received 'foo'
let typeOrLiteralErrByPath: Map<string, TypeOrLiteralErrByPathEntry> = new Map();
let typeOrLiteralErrByPath = new Map<string, TypeOrLiteralErrByPathEntry>();
for (const unionError of baseError.unionErrors.map((e) => e.errors).flat()) {
if (unionError.code === 'invalid_type' || unionError.code === 'invalid_literal') {
const flattenedErrorPath = flattenErrorPath(unionError.path);
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/content/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ export function getDataEntryExts(settings: Pick<AstroSettings, 'dataEntryTypes'>
export function getEntryConfigByExtMap<TEntryType extends ContentEntryType | DataEntryType>(
entryTypes: TEntryType[]
): Map<string, TEntryType> {
const map: Map<string, TEntryType> = new Map();
const map = new Map<string, TEntryType>();
for (const entryType of entryTypes) {
for (const ext of entryType.extensions) {
map.set(ext, entryType);
Expand Down Expand Up @@ -310,7 +310,7 @@ function getYAMLErrorLine(rawData: string | undefined, objectKey: string) {
return numNewlinesBeforeKey;
}

export function parseFrontmatter(fileContents: string, filePath: string) {
export function parseFrontmatter(fileContents: string) {
try {
// `matter` is empty string on cache results
// clear cache to prevent this
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ export class App {
): Promise<Response> {
const url = new URL(request.url);
const pathname = prependForwardSlash(this.removeBase(url.pathname));
const info = this.#routeDataToRouteInfo.get(routeData!)!;
const info = this.#routeDataToRouteInfo.get(routeData)!;
const isCompressHTML = this.#manifest.compressHTML ?? false;
// may be used in the future for handling rel=modulepreload, rel=icon, rel=manifest etc.
const links = new Set<never>();
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/app/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class NodeIncomingMessage extends IncomingMessage {
/**
* The read-only body property of the Request interface contains a ReadableStream with the body contents that have been added to the request.
*/
body?: any | undefined;
body?: unknown;
}

export class NodeApp extends App {
Expand Down
12 changes: 2 additions & 10 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export function chunkIsPage(
if (output.type !== 'chunk') {
return false;
}
const chunk = output as OutputChunk;
const chunk = output;
if (chunk.facadeModuleId) {
const facadeToEntryId = prependForwardSlash(
rootRelativeFacadeId(chunk.facadeModuleId, settings)
Expand Down Expand Up @@ -470,15 +470,7 @@ async function generatePath(
onRequest?: MiddlewareHandler<unknown>
) {
const { settings, logging, origin, routeCache } = opts;
const {
mod,
internals,
linkIds,
scripts: hoistedScripts,
styles: _styles,
pageData,
renderers,
} = gopts;
const { mod, internals, scripts: hoistedScripts, styles: _styles, pageData, renderers } = gopts;

// This adds the page name to the array so it can be shown as part of stats.
if (pageData.route.type === 'page') {
Expand Down
6 changes: 3 additions & 3 deletions packages/astro/src/core/build/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ export function registerAllPlugins({ internals, options, register }: AstroBuildP
register(pluginAliasResolve(internals));
register(pluginAnalyzer(internals));
register(pluginInternals(internals));
register(pluginRenderers(options, internals));
register(pluginMiddleware(options, internals));
register(pluginRenderers(options));
register(pluginMiddleware(options));
register(pluginPages(options, internals));
register(pluginCSS(options, internals));
register(astroHeadBuildPlugin(options, internals));
register(astroHeadBuildPlugin(internals));
register(pluginPrerender(options, internals));
register(astroConfigBuildPlugin(options, internals));
register(pluginHoistedScripts(options, internals));
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/build/plugins/plugin-analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin {

for (const id of ids) {
const info = this.getModuleInfo(id);
if (!info || !info.meta?.astro) continue;
if (!info?.meta?.astro) continue;

const astro = info.meta.astro as AstroPluginMetadata['astro'];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const astroEntryPrefix = '\0astro-entry:';
* entries to re-export only the names the user is using.
*/
export function vitePluginComponentEntry(internals: BuildInternals): VitePlugin {
const componentToExportNames: Map<string, string[]> = new Map();
const componentToExportNames = new Map<string, string[]>();

mergeComponentExportNames(internals.discoveredHydratedComponents);
mergeComponentExportNames(internals.discoveredClientOnlyComponents);
Expand Down
Loading