Skip to content

Commit

Permalink
Enable strictest TS settings and fix lints
Browse files Browse the repository at this point in the history
- exactOptionalPropertyTypes
- noPropertyAccessFromIndexSignature
- noImplicitOverride

This commit also cleans up deprecated lints, migrates older lints to
`n/` and sets up the import lint plugins needed for a good importing
experience.

There's a lot of changes in this commit, but most of it is minor tweaks
to deal with stylistic issues raised by lints.

The biggest substantive changes were required by
`exactOptionalPropertyTypes` and `noPropertyAccessFromIndexSignature`.
Both of these strict settings revealed many places where our code didn't
clearly handle missing values and nulls, and this commit adds the
necessary checks.

Some common patterns:

- Migrate C-style for loops to for/of loops, which eliminates nulls
- Pervasive use of the (already existing) `PresentArray` pattern and its
  utilities. Added `getFirst` and `getLast` utilities, as well as some
  variations on the present array checks.
- Used `?` to propagate undefined in testing situations where undefined
  would fail equality checks anyway.
  • Loading branch information
wycats committed May 5, 2023
1 parent a81c716 commit 64eb186
Show file tree
Hide file tree
Showing 310 changed files with 2,706 additions and 2,194 deletions.
504 changes: 250 additions & 254 deletions .eslintrc.json

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
"files.exclude": {
"**/.DS_Store": true,
"**/.git": true,
"dist/**": true,
"node_modules/**": true,
"**/node_modules/**": true,
"**/dist/**": true,
"tmp/**": true
},
"files.watcherExclude": {
Expand Down
4 changes: 2 additions & 2 deletions benchmark/benchmarks/krausest/lib/index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { createBenchmark } from '@glimmer/benchmark-env';

import ApplicationTemplate from './components/Application.hbs';
import Application from './components/Application';
import RowTemplate from './components/Row.hbs';
import ApplicationTemplate from './components/Application.hbs';
import Row from './components/Row';
import RowTemplate from './components/Row.hbs';
import buildData from './utils/data';

/**
Expand Down
1 change: 1 addition & 0 deletions benchmark/bin/build.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
const build = require('../lib/build');

build();
2 changes: 1 addition & 1 deletion benchmark/lib/build.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/* eslint-disable n/no-unpublished-require */
const path = require('path');
const rollup = require('rollup');
const sourcemap = require('rollup-plugin-sourcemaps');
const terser = require('@rollup/plugin-terser');
const strip = require('@rollup/plugin-strip');
const path = require('path');
const fs = require('fs-extra');
const symlinkOrCopy = require('symlink-or-copy').sync;

Expand Down
1 change: 1 addition & 0 deletions bin/run-node-tests.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import path from 'node:path';

import { execaSync } from 'execa';

const __dirname = new URL('.', import.meta.url).pathname;
Expand Down
3 changes: 2 additions & 1 deletion bin/sync-npm-owners.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
// glimmer-engine.

import { execSync } from 'node:child_process';
import { globSync } from 'glob';
import { readFileSync } from 'node:fs';
import { resolve } from 'node:path';

import { globSync } from 'glob';

const __dirname = new URL('.', import.meta.url).pathname;
const manifest = resolve(__dirname, '../package.json');
let name = JSON.parse(readFileSync(manifest)).name;
Expand Down
4 changes: 2 additions & 2 deletions build/broccoli/build-packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ const merge = require('broccoli-merge-trees');
const Rollup = require('broccoli-rollup');
const sourcemaps = require('rollup-plugin-sourcemaps');
const UnwatchedDir = require('broccoli-source').UnwatchedDir;
const debugMacros = require('babel-plugin-debug-macros');
const Project = require('../utils/project');
const transpileToES5 = require('./transpile-to-es5');
const writePackageJSON = require('./write-package-json');
const writeLicense = require('./write-license');
const debugMacros = require('babel-plugin-debug-macros');

const Project = require('../utils/project');
const project = Project.from('packages');

module.exports = function buildPackages(es2017, matrix) {
Expand Down
2 changes: 1 addition & 1 deletion build/broccoli/build-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ const path = require('path');
const merge = require('broccoli-merge-trees');
const funnel = require('broccoli-funnel');
const concat = require('broccoli-concat');
const transpileToES5 = require('./transpile-to-es5');
const babel = require('broccoli-babel-transpiler');
const transpileToES5 = require('./transpile-to-es5');

/**
* For development, this returns a Broccoli tree with:
Expand Down
2 changes: 1 addition & 1 deletion build/broccoli/write-license.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict';

const writeFile = require('broccoli-file-creator');
const { readFileSync } = require('fs');
const writeFile = require('broccoli-file-creator');

const LICENSE = readFileSync('./LICENSE', 'utf8');

Expand Down
4 changes: 3 additions & 1 deletion build/broccoli/write-smoke-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const Plugin = require('broccoli-plugin');
const fs = require('fs');
const path = require('path');
const Plugin = require('broccoli-plugin');

/**
* Writes a TypeScript file that imports each package. This file can be passed
Expand Down Expand Up @@ -33,6 +33,8 @@ class TypesSmokeTestWriter extends Plugin {
noUnusedLocals: true,
noUnusedParameters: true,
noImplicitReturns: true,
useDefineForClassFields: true,
exactOptionalPropertyTypes: true,

newLine: 'LF',
noEmit: true,
Expand Down
2 changes: 1 addition & 1 deletion build/debug.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// @ts-check

// eslint-disable-next-line n/no-missing-require
const { normalizeAll, buildEnum, buildMetas, strip } = require('../dist/@glimmer/debug');
const fs = require('fs');
const toml = require('toml');
const prettier = require('prettier');
const { normalizeAll, buildEnum, buildMetas, strip } = require('../dist/@glimmer/debug');

function parse(file) {
let opcodes = fs.readFileSync(file, { encoding: 'utf8' });
Expand Down
2 changes: 1 addition & 1 deletion build/utils/project.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
*/

const fs = require('fs');
const glob = require('glob');
const path = require('path');
const glob = require('glob');
const DAGMap = require('dag-map').default;

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/local-linker/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict';

const path = require('path');
const fs = require('fs-extra');
const symlinkOrCopy = require('symlink-or-copy').sync;
const path = require('path');

const rootDir = path.resolve(__dirname, '../..');

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
"eslint-config-prettier": "^8.8.0",
"eslint-import-resolver-typescript": "^3.5.5",
"eslint-plugin-import": "^2.27.5",
"eslint-plugin-unused-imports": "^2.0.0",
"eslint-plugin-n": "^15.7.0",
"eslint-plugin-prettier": "^4.2.1",
"eslint-plugin-qunit": "^7.3.4",
Expand Down
3 changes: 1 addition & 2 deletions packages/@glimmer/benchmark-env/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
export { default as createCell } from './src/create-cell';
export { default as createBenchmark } from './src/create-benchmark';

export { default as createCell } from './src/create-cell';
export { Benchmark, Cell, ComponentArgs, UpdateBenchmark } from './src/interfaces';
7 changes: 4 additions & 3 deletions packages/@glimmer/benchmark-env/src/benchmark/args-proxy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { CapturedNamedArguments, CapturedArguments } from '@glimmer/interfaces';
import { CapturedArguments, CapturedNamedArguments, Reference } from '@glimmer/interfaces';
import { valueForRef } from '@glimmer/reference';

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

class ArgsProxy implements ProxyHandler<CapturedNamedArguments> {
Expand All @@ -17,7 +18,7 @@ class ArgsProxy implements ProxyHandler<CapturedNamedArguments> {
): PropertyDescriptor | undefined {
let desc: PropertyDescriptor | undefined;
if (typeof p === 'string' && p in target) {
const value = valueForRef(target[p]);
const value = valueForRef(target[p] as Reference);
desc = {
enumerable: true,
configurable: false,
Expand All @@ -34,7 +35,7 @@ class ArgsProxy implements ProxyHandler<CapturedNamedArguments> {

get(target: CapturedNamedArguments, p: PropertyKey): any {
if (typeof p === 'string' && p in target) {
return valueForRef(target[p]);
return valueForRef(target[p] as Reference);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { WithCreateInstance, Dict, VMArguments, Template, Owner } from '@glimmer/interfaces';
import { Dict, Owner, Template, VMArguments, WithCreateInstance } from '@glimmer/interfaces';
import { getComponentTemplate } from '@glimmer/manager';
import { createConstRef, Reference } from '@glimmer/reference';
import { EMPTY_ARGS } from '@glimmer/runtime';
import { getComponentTemplate } from '@glimmer/manager';

import { ComponentArgs } from '../interfaces';
import argsProxy from './args-proxy';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { EnvironmentDelegate } from '@glimmer/runtime';
import { Destroyable, Destructor, RenderResult } from '@glimmer/interfaces';
import setGlobalContext from '@glimmer/global-context';
import { Destroyable, Destructor, RenderResult } from '@glimmer/interfaces';
import { EnvironmentDelegate } from '@glimmer/runtime';

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

Expand Down
14 changes: 7 additions & 7 deletions packages/@glimmer/benchmark-env/src/benchmark/create-registry.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
import {
ResolvedComponentDefinition,
InternalModifierManager,
Dict,
ModifierDefinitionState,
HelperDefinitionState,
Helper,
HelperDefinitionState,
InternalModifierManager,
ModifierDefinitionState,
ResolvedComponentDefinition,
SimpleElement,
} from '@glimmer/interfaces';
import { programCompilationContext } from '@glimmer/opcode-compiler';
import { artifacts, RuntimeOpImpl } from '@glimmer/program';
import { SimpleElement } from '@glimmer/interfaces';
import {
getComponentTemplate,
getInternalComponentManager,
setInternalHelperManager,
setInternalModifierManager,
} from '@glimmer/manager';
import { programCompilationContext } from '@glimmer/opcode-compiler';
import { artifacts, RuntimeOpImpl } from '@glimmer/program';

import { UpdateBenchmark } from '../interfaces';
import renderBenchmark from './render-benchmark';
Expand Down
8 changes: 6 additions & 2 deletions packages/@glimmer/benchmark-env/src/benchmark/on-modifier.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { CapturedArguments, InternalModifierManager, Owner } from '@glimmer/interfaces';
import {
CapturedArguments,
InternalModifierManager,
Owner,
SimpleElement,
} from '@glimmer/interfaces';
import { Reference, valueForRef } from '@glimmer/reference';
import { castToBrowser } from '@glimmer/util';
import { createUpdatableTag } from '@glimmer/validator';
import { SimpleElement } from '@glimmer/interfaces';

interface OnModifierState {
element: SimpleElement;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { SimpleElement } from '@glimmer/interfaces';
import {
CompileTimeCompilationContext,
Dict,
RuntimeResolver,
ResolvedComponentDefinition,
RuntimeArtifacts,
CompileTimeCompilationContext,
RuntimeResolver,
SimpleElement,
} from '@glimmer/interfaces';
import { NewElementBuilder, runtimeContext, renderComponent, renderSync } from '@glimmer/runtime';
import { NewElementBuilder, renderComponent, renderSync, runtimeContext } from '@glimmer/runtime';

import { UpdateBenchmark } from '../interfaces';
import createEnvDelegate, { registerResult } from './create-env-delegate';
import { measureRender } from './util';
import { UpdateBenchmark } from '../interfaces';

export default async function renderBenchmark(
artifacts: RuntimeArtifacts,
Expand Down
3 changes: 1 addition & 2 deletions packages/@glimmer/benchmark-env/src/benchmark/util.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { CompileTimeComponent } from '@glimmer/interfaces';
import { CompileTimeCompilationContext, CompileTimeComponent } from '@glimmer/interfaces';
import { unwrapHandle } from '@glimmer/util';
import { CompileTimeCompilationContext } from '@glimmer/interfaces';

export function compileEntry(entry: CompileTimeComponent, context: CompileTimeCompilationContext) {
return unwrapHandle(entry.compilable!.compile(context));
Expand Down
9 changes: 5 additions & 4 deletions packages/@glimmer/benchmark-env/src/create-benchmark.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { setComponentTemplate, setInternalComponentManager } from '@glimmer/manager';
import { Benchmark } from './interfaces';
import createRegistry from './benchmark/create-registry';
import onModifier from './benchmark/on-modifier';
import basicComponentManager from './benchmark/basic-component-manager';
import { templateFactory } from '@glimmer/opcode-compiler';
import { templateOnlyComponent } from '@glimmer/runtime';

import basicComponentManager from './benchmark/basic-component-manager';
import createRegistry from './benchmark/create-registry';
import onModifier from './benchmark/on-modifier';
import { Benchmark } from './interfaces';

export default function createBenchmark(): Benchmark {
const registry = createRegistry();
registry.registerModifier('on', {}, onModifier);
Expand Down
5 changes: 3 additions & 2 deletions packages/@glimmer/benchmark-env/src/create-cell.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {
tagMetaFor,
consumeTag,
dirtyTagFor,
TagMeta,
tagFor,
TagMeta,
tagMetaFor,
UpdatableTag,
} from '@glimmer/validator';

import { Cell } from './interfaces';

class CellImpl<T> implements Cell<T> {
Expand Down
3 changes: 1 addition & 2 deletions packages/@glimmer/benchmark-env/src/interfaces.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Dict, SerializedTemplateWithLazyBlock } from '@glimmer/interfaces';
import { SimpleElement } from '@glimmer/interfaces';
import { Dict, SerializedTemplateWithLazyBlock, SimpleElement } from '@glimmer/interfaces';

/**
* This abstracts a tracked root.
Expand Down
24 changes: 9 additions & 15 deletions packages/@glimmer/compiler/lib/builder/builder-interface.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Dict, DictValue, Option, PresentArray } from '@glimmer/interfaces';
import { assertNever, dict, expect, isPresent } from '@glimmer/util';
import { assertNever, dict, expect, isPresentArray } from '@glimmer/util';

export type BuilderParams = BuilderExpression[];
export type BuilderHash = Option<Dict<BuilderExpression>>;
Expand Down Expand Up @@ -314,7 +314,7 @@ function extractBlockHead(name: string): NormalizedHead {
throw new Error(`Unexpected missing # in block head`);
}

return normalizeDottedPath(result[2]);
return normalizeDottedPath(result[2] as string);
}

function normalizeCallHead(name: string): NormalizedHead {
Expand All @@ -324,13 +324,13 @@ function normalizeCallHead(name: string): NormalizedHead {
throw new Error(`Unexpected missing () in call head`);
}

return normalizeDottedPath(result[1]);
return normalizeDottedPath(result[1] as string);
}

function normalizePath(head: string, tail: string[] = []): NormalizedHead {
let pathHead = normalizePathHead(head);

if (isPresent(tail)) {
if (isPresentArray(tail)) {
return {
type: ExpressionKind.GetPath,
path: {
Expand All @@ -349,11 +349,11 @@ function normalizePath(head: string, tail: string[] = []): NormalizedHead {
function normalizeDottedPath(whole: string): NormalizedHead {
let { kind, name: rest } = normalizePathHead(whole);

let [name, ...tail] = rest.split('.');
let [name, ...tail] = rest.split('.') as [string, ...string[]];

let variable: Variable = { kind, name, mode: 'loose' };

if (isPresent(tail)) {
if (isPresentArray(tail)) {
return { type: ExpressionKind.GetPath, path: { head: variable, tail } };
} else {
return { type: ExpressionKind.GetVar, variable };
Expand Down Expand Up @@ -503,12 +503,12 @@ function normalizeAttr(attr: BuilderAttr): { expr: NormalizedAttr; trusted: bool

function mapObject<T extends Dict<unknown>, Out>(
object: T,
callback: (value: DictValue<T>, key: keyof T) => Out
mapper: (value: DictValue<T>, key: keyof T) => Out
): { [P in keyof T]: Out } {
let out = dict() as { [P in keyof T]?: Out };

Object.keys(object).forEach(<K extends keyof T>(k: K) => {
out[k] = callback(object[k] as DictValue<T>, k);
out[k] = mapper(object[k] as DictValue<T>, k);
});

return out as { [P in keyof T]: Out };
Expand Down Expand Up @@ -537,13 +537,7 @@ export function isElement(input: [string, ...unknown[]]): input is BuilderElemen
export function extractElement(input: string): Option<string> {
let match = /^<([a-z0-9\-][a-zA-Z0-9\-]*)>$/.exec(input);

return match ? match[1] : null;
}

export function extractAngleInvocation(input: string): Option<string> {
let match = /^<(@[a-zA-Z0-9]*|[A-Z][a-zA-Z0-9\-]*)>$/.exec(input[0]);

return match ? match[1] : null;
return match?.[1] ?? null;
}

export function isAngleInvocation(input: [string, ...unknown[]]): input is InvocationElement {
Expand Down
Loading

0 comments on commit 64eb186

Please sign in to comment.