Skip to content

Commit

Permalink
Enables eslint on the full repo and adds a rule for no only() tests (
Browse files Browse the repository at this point in the history
…#3659)

* enabling eslint on the all packages and tests

* enabling for all packages

* TEMP: adding an only() test to verify it fails CI

* using our eslint config and ignore in CI

* removing the temporary .only() test

* update lock file

* lint: fixing new test with a no-shadow warning

* chore: update lock file
  • Loading branch information
Tony Sullivan authored Jun 22, 2022
1 parent f6400e6 commit b8c6dab
Show file tree
Hide file tree
Showing 23 changed files with 61 additions and 43 deletions.
11 changes: 6 additions & 5 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
**/*.js
**/*.ts
!packages/astro/**/*.js
!packages/astro/**/*.ts
packages/astro/test/**/*.js
**/*.d.ts
packages/**/dist/**/*
packages/**/fixtures/**/*
packages/webapi/**/*
packages/astro/vendor/vite/**/*
examples/**/*
scripts/**/*
.github
.changeset
3 changes: 2 additions & 1 deletion .eslintrc.cjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module.exports = {
parser: '@typescript-eslint/parser',
extends: ['plugin:@typescript-eslint/recommended', 'prettier'],
plugins: ['@typescript-eslint', 'prettier'],
plugins: ['@typescript-eslint', 'prettier', 'no-only-tests'],
rules: {
'@typescript-eslint/ban-ts-comment': 'off',
'@typescript-eslint/camelcase': 'off',
Expand All @@ -17,5 +17,6 @@ module.exports = {
'prefer-const': 'off',
'no-shadow': 'off',
'@typescript-eslint/no-shadow': ['error'],
'no-only-tests/no-only-tests': 'error'
},
};
3 changes: 0 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
eslint: true
eslint_args: --ignore-pattern test --ignore-pattern vendor
eslint_dir: packages/astro
eslint_extensions: ts
prettier: false
auto_fix: true
git_name: github-actions[bot]
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,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": "turbo run benchmark --scope=astro",
"lint": "eslint \"packages/**/*.ts\"",
"lint": "eslint .",
"version": "changeset version && pnpm install --no-frozen-lockfile && pnpm run format"
},
"workspaces": [
Expand Down Expand Up @@ -69,6 +69,7 @@
"esbuild": "^0.14.42",
"eslint": "^8.16.0",
"eslint-config-prettier": "^8.5.0",
"eslint-plugin-no-only-tests": "^2.6.0",
"eslint-plugin-prettier": "^4.0.0",
"execa": "^6.1.0",
"organize-imports-cli": "^0.10.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/e2e/ts-resolution.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ const test = base.extend({
},
});

function runTest(test) {
test('client:idle', async ({ page, astro }) => {
function runTest(it) {
it('client:idle', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/'));

const counter = page.locator('#client-idle');
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/test/astro-css-bundling.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ describe('CSS Bundling', function () {

// test 3: assert all bundled CSS was built and contains CSS
for (const url of builtCSS.keys()) {
const css = await fixture.readFile(url);
expect(css).to.be.ok;
const bundledCss = await fixture.readFile(url);
expect(bundledCss).to.be.ok;
}
}
});
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/test/sass.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('Sass', () => {
// TODO: Sass cannot be found on macOS for some reason... Vite issue?
const test = os.platform() === 'darwin' ? it.skip : it;
test('shows helpful error on failure', async () => {
const res = await fixture.fetch('/error').then((res) => res.text());
expect(res).to.include('Undefined variable');
const text = await fixture.fetch('/error').then((res) => res.text());
expect(text).to.include('Undefined variable');
});
});
4 changes: 2 additions & 2 deletions packages/astro/test/ssr-adapter-build-config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ describe('Integration buildConfig hook', () => {
if (id === '@my-ssr') {
return id;
} else if (id === 'astro/app') {
const id = viteID(new URL('../dist/core/app/index.js', import.meta.url));
return id;
const viteId = viteID(new URL('../dist/core/app/index.js', import.meta.url));
return viteId;
}
},
load(id) {
Expand Down
3 changes: 1 addition & 2 deletions packages/astro/test/static-build-page-dist-url.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ import { expect } from 'chai';
import { loadFixture } from './test-utils.js';

describe('Static build: pages routes have distURL', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
/** @type {RouteData[]} */
let checkRoutes;
before(async () => {
/** @type {import('./test-utils').Fixture} */
const fixture = await loadFixture({
root: './fixtures/astro pages/',
integrations: [
Expand Down
10 changes: 4 additions & 6 deletions packages/astro/test/tailwindcss.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import { expect } from 'chai';
import * as cheerio from 'cheerio';
import { loadFixture } from './test-utils.js';

let fixture;

describe('Tailwind', () => {
let fixture;

Expand Down Expand Up @@ -62,10 +60,10 @@ describe('Tailwind', () => {

it('handles Markdown pages', async () => {
const html = await fixture.readFile('/markdown-page/index.html');
const $ = cheerio.load(html);
const bundledCSSHREF = $('link[rel=stylesheet][href^=/assets/]').attr('href');
const bundledCSS = await fixture.readFile(bundledCSSHREF.replace(/^\/?/, '/'));
expect(bundledCSS, 'includes used component classes').to.match(/\.bg-purple-600{/);
const $md = cheerio.load(html);
const bundledCSSHREF = $md('link[rel=stylesheet][href^=/assets/]').attr('href');
const mdBundledCSS = await fixture.readFile(bundledCSSHREF.replace(/^\/?/, '/'));
expect(mdBundledCSS, 'includes used component classes').to.match(/\.bg-purple-600{/);
});
});
});
4 changes: 2 additions & 2 deletions packages/astro/test/test-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ export default function () {
if (id === '@my-ssr') {
return id;
} else if (id === 'astro/app') {
const id = viteID(new URL('../dist/core/app/index.js', import.meta.url));
return id;
const viteId = viteID(new URL('../dist/core/app/index.js', import.meta.url));
return viteId;
}
},
load(id) {
Expand Down
1 change: 1 addition & 0 deletions packages/create-astro/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint no-console: 'off' */
import degit from 'degit';
import { execa, execaCommand } from 'execa';
import fs from 'fs';
Expand Down
4 changes: 2 additions & 2 deletions packages/create-astro/test/directory-step.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { resolve } from 'path';
import path from 'path';
import { promises, existsSync } from 'fs';
import { PROMPT_MESSAGES, testDir, setup, promiseWithTimeout, timeout } from './utils.js';

Expand Down Expand Up @@ -31,7 +31,7 @@ describe('[create-astro] select directory', function () {
});
});
it('should proceed on an empty directory', async function () {
const resolvedEmptyDirPath = resolve(testDir, inputs.emptyDir);
const resolvedEmptyDirPath = path.resolve(testDir, inputs.emptyDir);
if (!existsSync(resolvedEmptyDirPath)) {
await promises.mkdir(resolvedEmptyDirPath);
}
Expand Down
1 change: 1 addition & 0 deletions packages/integrations/deno/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export function start(manifest: SSRManifest, options: Options) {
});

_startPromise = Promise.resolve(_server.listenAndServe());
// eslint-disable-next-line no-console
console.error(`Server running on port ${port}`);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Deno.test({
const div = doc.querySelector('#thing');
assert(div, 'div exists');
} catch (err) {
// eslint-disable-next-line no-console
console.error(err);
} finally {
await close();
Expand Down
4 changes: 2 additions & 2 deletions packages/integrations/partytown/src/sirv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ export default function (dir, opts = {}) {
});
}

let lookup = opts.dev ? viaLocal.bind(0, dir, isEtag) : viaCache.bind(0, FILES);
let fileLookup = opts.dev ? viaLocal.bind(0, dir, isEtag) : viaCache.bind(0, FILES);

return function (req, res, next) {
let extns = [''];
Expand All @@ -224,7 +224,7 @@ export default function (dir, opts = {}) {
}

let data =
lookup(pathname, extns) || (isSPA && !isMatch(pathname, ignores) && lookup(fallback, extns));
fileLookup(pathname, extns) || (isSPA && !isMatch(pathname, ignores) && fileLookup(fallback, extns));
if (!data) return next ? next() : isNotFound(req, res);

if (isEtag && req.headers['if-none-match'] === data.headers['ETag']) {
Expand Down
2 changes: 1 addition & 1 deletion packages/integrations/sitemap/src/utils/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export class Logger implements ILogger {
this.packageName = packageName;
}

private log(msg: string, prefix: string = '') {
private log(msg: string, prefix = '') {
// eslint-disable-next-line no-console
console.log(`%s${this.packageName}:%s ${msg}\n`, prefix, prefix ? this.colors.reset : '');
}
Expand Down
2 changes: 1 addition & 1 deletion packages/integrations/sitemap/src/validate-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const validateOptions = (site: string | undefined, opts: SitemapOptions)
site: z.string().optional(), // Astro takes care of `site`: how to validate, transform and refine
canonicalURL: z.string().optional(), // `canonicalURL` is already validated in prev step
})
.refine(({ site, canonicalURL }) => site || canonicalURL, {
.refine((options) => options.site || options.canonicalURL, {
message: 'Required `site` astro.config option or `canonicalURL` integration option',
})
.parse({
Expand Down
2 changes: 1 addition & 1 deletion packages/integrations/vercel/src/lib/fs.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { PathLike } from 'node:fs';
import * as fs from 'node:fs/promises';

export async function writeJson<T extends any>(path: PathLike, data: T) {
export async function writeJson<T>(path: PathLike, data: T) {
await fs.writeFile(path, JSON.stringify(data), { encoding: 'utf-8' });
}

Expand Down
9 changes: 5 additions & 4 deletions packages/markdown/remark/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ export async function renderMarkdown(
const loadedRemarkPlugins = await Promise.all(loadPlugins(remarkPlugins));
const loadedRehypePlugins = await Promise.all(loadPlugins(rehypePlugins));

loadedRemarkPlugins.forEach(([plugin, opts]) => {
parser.use([[plugin, opts]]);
loadedRemarkPlugins.forEach(([plugin, pluginOpts]) => {
parser.use([[plugin, pluginOpts]]);
});

if (scopedClassName) {
Expand Down Expand Up @@ -87,8 +87,8 @@ export async function renderMarkdown(
],
]);

loadedRehypePlugins.forEach(([plugin, opts]) => {
parser.use([[plugin, opts]]);
loadedRehypePlugins.forEach(([plugin, pluginOpts]) => {
parser.use([[plugin, pluginOpts]]);
});

parser
Expand All @@ -106,6 +106,7 @@ export async function renderMarkdown(
// Ensure that the error message contains the input filename
// to make it easier for the user to fix the issue
err = prefixError(err, `Failed to parse Markdown file "${input.path}"`);
// eslint-disable-next-line no-console
console.error(err);
throw err;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/markdown/remark/src/rehype-collect-headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export default function createCollectHeaders() {

let text = '';
let isJSX = false;
visit(node, (child, _, parent) => {
visit(node, (child, __, parent) => {
if (child.type === 'element' || parent == null) {
return;
}
Expand Down
7 changes: 4 additions & 3 deletions packages/markdown/remark/src/remark-prism.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ function runHighlighter(lang: string, code: string) {
lang = 'plaintext';
}

const ensureLoaded = (lang: string) => {
if (lang && !Prism.languages[lang]) {
loadLanguages([lang]);
const ensureLoaded = (language: string) => {
if (language && !Prism.languages[language]) {
loadLanguages([language]);
}
};

Expand All @@ -30,6 +30,7 @@ function runHighlighter(lang: string, code: string) {
}

if (lang && !Prism.languages[lang]) {
// eslint-disable-next-line no-console
console.warn(`Unable to load the language: ${lang}`);
}

Expand Down
16 changes: 16 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit b8c6dab

Please sign in to comment.