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

Extract formatting / lint changes from the Error Recovery PR, and add prettier to ci / local tooling #1500

Merged
merged 6 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
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
3 changes: 2 additions & 1 deletion 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:format": "prettier -c .",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

formatting wasn't being checked on C.I.... 🙃

"lint:files": "dotenv -- turbo lint",
"force:lint:files": "eslint .",
"lint:types": "tsc -b",
"lint:fix": "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
Loading