Skip to content

Commit

Permalink
Extract formatting / lint changes from the Error Recovery PR, and add…
Browse files Browse the repository at this point in the history
… prettier to ci / local tooling (#1500)

* Run prettier with lint:fix

* Update lockfile

* Remove dotenv from lint:files, this busts the turbo cache and makes the linting job slow

* Disable a lint on one additional file

* Don't bust the cache for turbo based on if is logging warnings or not (NODE_OPTIONS)

* Addition of lint:fix for ci was accidentally running when lint was running, doubling up on lint tasks
  • Loading branch information
NullVoxPopuli authored Nov 22, 2023
1 parent 8a84e2f commit c66e4dd
Show file tree
Hide file tree
Showing 247 changed files with 1,512 additions and 1,248 deletions.
13 changes: 13 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

node_modules/

# output directories
dist/
ts-dist/


# We don't need prettier here
*.md
*.yaml
*.yml
guides/
26 changes: 23 additions & 3 deletions .prettierrc.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
{
"singleQuote": true,
"trailingComma": "es5",
"printWidth": 100
"printWidth": 100,
"plugins": [],
"overrides": [
{
"files": ["**/*.{js,ts,cjs,mjs,cts,mts,cts}"],
"options": {
"singleQuote": true,
"trailingComma": "es5"
}
},
{
"files": ["*.json"],
"options": {
"singleQuote": false
}
},
{
"files": ["*.hbs"],
"options": {
"singleQuote": false
}
}
]
}
2 changes: 1 addition & 1 deletion .watchmanconfig
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"fsevents_latency": 0.02,
"ignore_dirs": [ "tmp" ]
"ignore_dirs": ["tmp"]
}
2 changes: 1 addition & 1 deletion benchmark/benchmarks/krausest/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<title>krausest benchmark</title>
</head>
<script type="module">
import run from './browser.js';
import run from "./browser.js";
run();
</script>
<body></body>
Expand Down
6 changes: 3 additions & 3 deletions benchmark/benchmarks/krausest/lib/components/Application.hbs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div class="container">
<table class="table table-hover table-striped test-data"><tbody>
{{#each @items as |item|}}<Row @select={{this.select}} @item={{item}}/>{{/each}}
</tbody></table>
{{#each @items as |item|}}<Row @select={{this.select}} @item={{item}} />{{/each}}
</tbody></table>
<span className="preloadicon glyphicon glyphicon-remove" aria-hidden="true"></span>
</div>
</div>
7 changes: 5 additions & 2 deletions benchmark/benchmarks/krausest/lib/components/Row.hbs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
<tr class={{if @item.selected "danger"}}>
<td class="col-md-1">{{@item.id}}</td>
<td class="col-md-4"><a {{on "click" this.onSelect}}>{{@item.label}}</a></td>
<td class="col-md-1"><a><span class="glyphicon glyphicon-remove" aria-hidden="true"></span></a></td>
<td class="col-md-1"><a><span
class="glyphicon glyphicon-remove"
aria-hidden="true"
></span></a></td>
<td class="col-md-6"></td>
</tr>
</tr>
1 change: 0 additions & 1 deletion benchmark/bin/control.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

const express = require('express');

const app = express();
Expand Down
1 change: 0 additions & 1 deletion benchmark/bin/experiment.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

const express = require('express');

const app = express();
Expand Down
2 changes: 0 additions & 2 deletions benchmark/lib/build.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// @ts-check


const path = require('path');
const rollup = /** @type {{rollup: import("rollup").rollup}} */ (
/** @type {unknown} */
Expand Down Expand Up @@ -42,7 +41,6 @@ async function build(dist, out) {
}),
terser({
compress: {

negate_iife: false,
sequences: 0,
},
Expand Down
15 changes: 8 additions & 7 deletions bin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,18 @@
"name": "@glimmer-workspace/bin",
"private": true,
"dependencies": {
"puppeteer-chromium-resolver": "^20.0.0",
"chalk": "^5.2.0",
"execa": "^7.1.1",
"js-yaml": "^4.1.0",
"glob": "^10.2.3",
"@types/glob": "^8.1.0",
"@types/js-yaml": "^4.0.5",
"@types/node": "^18.16.7",
"@types/puppeteer-chromium-resolver": "workspace:^"
"@types/puppeteer-chromium-resolver": "workspace:^",
"chalk": "^5.2.0",
"execa": "^7.1.1",
"glob": "^10.2.3",
"js-yaml": "^4.1.0",
"puppeteer-chromium-resolver": "^20.0.0"
},
"devDependencies": {
"eslint": "^8.54.0",
"esno": "^0.16.3"
},
"engines": {
Expand All @@ -25,4 +26,4 @@
"test:lint": "eslint .",
"test:types": "tsc --noEmit -p ./tsconfig.json"
}
}
}
10 changes: 5 additions & 5 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,19 @@
// Do this first so that `@glimmer-workspace/integration-tests` will see the global
// `QUnit`. We should change the code structure here so that we don't
// rely on the global to bootstrap QUnit, but this is fine for now.
const QUnit = await import('qunit');
const QUnit = await import("qunit");
QUnit.config.autostart = false;
</script>
<script type="module">
import { setupQunit } from '@glimmer-workspace/integration-tests';
import { setupQunit } from "@glimmer-workspace/integration-tests";
const { smokeTest } = await setupQunit();

const packages = await import.meta.glob('./packages/@glimmer/*/test/**/*-test.ts');
const packages = await import.meta.glob("./packages/@glimmer/*/test/**/*-test.ts");
const integrationTestFiles = await import.meta.glob(
'./packages/@glimmer-workspace/*/test/**/*-test.ts'
"./packages/@glimmer-workspace/*/test/**/*-test.ts"
);

let smokeTestFile = './packages/@glimmer-workspace/integration-tests/test/smoke-test.ts';
let smokeTestFile = "./packages/@glimmer-workspace/integration-tests/test/smoke-test.ts";

// evaluate the tests before starting QUnit
for (const [name, pkg] of Object.entries(packages)) {
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@
"build:control": "rollup -c rollup.config.mjs",
"build:flags": "RETAIN_FLAGS=true ember build --env production --suppress-sizes",
"lint": "npm-run-all lint:*",
"lint:files": "dotenv -- turbo lint",
"force:lint:files": "eslint .",
"lint:format": "prettier -c .",
"lint:files": "turbo lint",
"lint:types": "tsc -b",
"lintfix": "pnpm turbo test:lint -- --fix && prettier -w .",
"start": "ember serve --port=7357",
"test": "node bin/run-tests.mjs",
"test:browserstack": "ember test --test-port=7774 --host 127.0.0.1 --config-file=testem-browserstack.js",
Expand Down
150 changes: 125 additions & 25 deletions packages/@glimmer-workspace/.eslintrc.cjs
Original file line number Diff line number Diff line change
@@ -1,67 +1,167 @@
const { resolve } = require('path');
const { existsSync, readFileSync } = require('fs');
const { resolve, relative, dirname } = require('path');

const tsconfig = resolve(__dirname, 'tsconfig.json');
const glob = require('fast-glob');

const libs = glob.sync(['tsconfig.json', '*/tsconfig.json'], {
cwd: __dirname,
absolute: true,
});

const tests = glob.sync(['tsconfig.json', '*/test/tsconfig.json'], {
cwd: __dirname,
absolute: true,
});

const testDirs = glob.sync(['*/test'], {
cwd: __dirname,
absolute: true,
onlyDirectories: true,
ignore: ['**/node_modules/**'],
});

/**
* @type {Record<string, Record<string, string[] | undefined>>}
*/
const RESTRICTIONS = {};

for (const dir of testDirs) {
const pkgRoot = dirname(dir);

if (!existsSync(resolve(dir, 'package.json'))) {
// don't create rules if the tests dir doesn't have a package.json
continue;
}

const packageName = JSON.parse(readFileSync(resolve(pkgRoot, 'package.json'), 'utf8')).name;

const files = glob.sync(['**/*.{js,ts,d.ts}'], {
cwd: dir,
absolute: true,
onlyFiles: true,
ignore: ['**/node_modules/**'],
});

for (const file of files) {
const relativeFile = `./${relative(__dirname, file)}`;
const relativeToLib = relative(dirname(file), resolve(dirname(dir), 'lib'));

const pkg = (RESTRICTIONS[packageName] ??= {});

if (pkg[relativeToLib]) {
pkg[relativeToLib].push(relativeFile);
} else {
pkg[relativeToLib] = [relativeFile];
}
}
}

/**
* @type {import("eslint").Linter.Config["overrides"]}
*/
const overrides = Object.entries(RESTRICTIONS).flatMap(([pkgName, overrides]) =>
Object.entries(overrides).map(([lib, files]) => ({
files,
rules: {
'@typescript-eslint/no-restricted-imports': [
'error',
{
paths: [
{
name: lib,
message: `Import from ${pkgName} instead`,
},
],
},
],
},
}))
);

// console.dir({ overrides }, { depth: null });

/** @type {import("eslint").Linter.Config} */
module.exports = {
root: false,
overrides: [
{
files: ['*/index.{js,ts,d.ts}', '*/lib/**/*.{js,ts,d.ts}', '*/test/**/*.{js,ts,d.ts}'],
files: ['*/lib/**/*.{js,cjs,mjs,ts,d.ts}'],
excludedFiles: ['node_modules', '*/node_modules'],
parserOptions: {
project: libs,
},
plugins: ['@glimmer-workspace'],
extends: ['plugin:@glimmer-workspace/recommended'],
},
{
files: ['*/lib/**/*.cjs'],
env: {
node: true,
},
excludedFiles: ['node_modules', '*/node_modules'],
parserOptions: {
ecmaVersion: 'latest',
project: [tsconfig],
project: libs,
},
plugins: ['@glimmer-workspace'],
extends: ['plugin:@glimmer-workspace/recommended'],
rules: {
'@typescript-eslint/no-require-imports': 'off',
'@typescript-eslint/no-var-requires': 'off',
'no-undef': 'off',
},
},
{
files: ['./integration-tests/{lib,test}/**/*.ts'],
rules: {
// off for now
'@typescript-eslint/no-explicit-any': 'off',
},
},
// QUnit is a weird package, and there are some issues open about fixing it
// - https://github.com/qunitjs/qunit/issues/1729
// - https://github.com/qunitjs/qunit/issues/1727
// - https://github.com/qunitjs/qunit/issues/1724
{
files: ['**/*-test.ts', '**/{test,integration-tests}/**/*.ts'],
files: ['*/test/**/*.{js,ts,d.ts}'],
parserOptions: {
project: tests,
},
plugins: ['@glimmer-workspace'],
extends: ['plugin:@glimmer-workspace/recommended'],
rules: {
'@typescript-eslint/unbound-method': 'off',
// off for now
'@typescript-eslint/no-explicit-any': 'off',
'import/no-relative-packages': 'error',
'no-restricted-paths': 'off',
},
},
//////////////////////////////////////////////////////////
// Remove when https://github.com/glimmerjs/glimmer-vm/pull/1462
// is merged.
//////////////////////////////////////////////////////////
...overrides,
// TODO: / CLEANUP: / TECHDEBT:
{
files: [
'integration-tests/lib/dom/assertions.ts',
'integration-tests/lib/snapshot.ts',
'integration-tests/lib/dom/simple-utils.ts',
'integration-tests/lib/modes/rehydration/builder.ts',
'integration-tests/lib/snapshot.ts',
],
rules: {
'@typescript-eslint/no-unsafe-enum-comparison': 'off',
'@typescript-eslint/no-unsafe-enum-comparison': 'warn',
},
},
{
files: [
'integration-tests/test/updating-test.ts',
'integration-tests/lib/dom/simple-utils.ts',
'integration-tests/lib/modes/jit/delegate.ts',
'integration-tests/lib/modes/jit/register.ts',
'integration-tests/lib/render-delegate.ts',
'integration-tests/test/style-warnings-test.ts',
'integration-tests/test/updating-content-matrix-test.ts',
'integration-tests/test/ember-component-test.ts',
'integration-tests/lib/render-test.ts',
],
rules: {
'@typescript-eslint/no-redundant-type-constituents': 'off',
'@typescript-eslint/no-base-to-string': 'warn',
},
},
{
files: [
'integration-tests/test/ember-component-test.ts',
'integration-tests/lib/render-test.ts',
],
files: ['integration-tests/lib/suites/each.ts', 'integration-tests/lib/dom/assertions.ts'],
rules: {
'@typescript-eslint/no-base-to-string': 'off',
'deprecation/deprecation': 'warn',
},
},
],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import type {
Dict,
Owner,
Template,
VMArguments,
WithCreateInstance,
} from "@glimmer/interfaces";
import type { Dict, Owner, Template, VMArguments, WithCreateInstance } from '@glimmer/interfaces';
import type { Reference } from '@glimmer/reference';
import { getComponentTemplate } from '@glimmer/manager';
import { createConstRef, type Reference } from '@glimmer/reference';
import { createConstRef } from '@glimmer/reference';
import { EMPTY_ARGS } from '@glimmer/runtime';

import type { ComponentArgs } from '../interfaces';

import argsProxy from './args-proxy';

const BASIC_COMPONENT_CAPABILITIES = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/* eslint-disable no-console */

import setGlobalContext from '@glimmer/global-context';
import type { Destroyable, Destructor, RenderResult } from '@glimmer/interfaces';
import type { EnvironmentDelegate } from '@glimmer/runtime';
import setGlobalContext from '@glimmer/global-context';

type Queue = (() => void)[];

Expand Down
Loading

0 comments on commit c66e4dd

Please sign in to comment.