Skip to content

Commit

Permalink
chore: enable Typescript's strict mode (#648)
Browse files Browse the repository at this point in the history 
* chore(typescript): enable strict mode

* chore(typescript): ignore pixi plugin code

* fix(utils): fix/escape typescript errors

* chore(tests): add optional chaining to accessors

* chore(components): fix/escape typescript errors

Mostly ignoring undefined errors using `!`, to speed up transitioning to
strict types, and fixing the actual types on ad-hoc basis.

* chore(base-layers): fix typescript errors related to undefuned/nulls

* fix(layers-displayobjects): fix/escape typescript undefined errors

* chore(layers): fix/escape typescript errors

* chore(InterSectionReferenceSystem): remove unused properties

* chore(GeomodelLabelsLayer): fix typescript optional parameters

* fix(eslint): run eslint --fix on the codebase

* docs(eslint): add comment with regard to null-assertion warnings

* fix: swap null check and isFinite check

* Create thin-experts-confess.md

* Update thin-experts-confess.md
  • Loading branch information
@venikx
venikx authored Jun 26, 2023
1 parent 53386e9 commit ef0590b
Show file tree
Hide file tree
Showing 51 changed files with 985 additions and 903 deletions.
5 changes: 5 additions & 0 deletions .changeset/thin-experts-confess.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@equinor/esv-intersection": patch
---

chore: enable Typescript's strict mode & fix 1500 errors
49 changes: 29 additions & 20 deletions .eslintrc.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,36 @@
"SharedArrayBuffer": "readonly"
},
"rules": {
"prettier/prettier": "error",

"@typescript-eslint/no-explicit-any": "error",
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/no-inferrable-types": "off",
"@typescript-eslint/ban-ts-comment": "warn",
"@typescript-eslint/no-namespace": "warn",
"@typescript-eslint/no-unused-vars": ["error", { "args": "all", "varsIgnorePattern": "^_", "argsIgnorePattern": "^_" }],
"@typescript-eslint/no-use-before-define": "off",

"curly": "error",
"no-continue": "off",
"no-plusplus": "off",
"no-param-reassign": "off",
"object-curly-newline": "off",
"no-underscore-dangle": "off",
"quotes": ["error", "single"],
"import/prefer-default-export": "off",
"max-len": ["error", { "code": 150 }],
"comma-dangle": ["error", "always-multiline"],
"eqeqeq": ["error", "always", { "null": "ignore" }],
"no-magic-numbers": ["error", { "ignore": [-1, 0, 0.5, 1, 2, 3, 4, 8, 16, 100, 255, 1000], "ignoreDefaultValues": true }],
"newline-per-chained-call": ["off", { "ignoreChainWithDepth": 1 }],
"no-warning-comments": [0, { "terms": ["todo", "fixme"], "location": "start" }]
/* NOTE(Kevin): Currently needed after enabling strict mode, as there were over 1500 Typescript errors.
*
* Fixing all these errors properly, would
* 1) result in a necessary api changes with regards to most of the public interfaces
* 2) require drastic changes in implementation details, with an even higher risk
* of introducing regressions.
*
* Therefore I opted to make minimal implementation detail changes in attempt to not
* break existing code, while still adhering to Typescript's strict mode.
* But why enable strict mode?
* We've seen a couple bugs now that could have been prevented with strict mode. So
* everywhere in the code where there isn't heavily escaped code with `?` or `!` to
* force a `fake, correct type` the Typescript compiler will catch those mistakes.
* However, type mismatches might still happen in these cases where there's heavy use
* of `?` and `!` and those typing mistakes were already existing bugs, that were
* waiting to be fixed regardless. Once these bug surface in these areas, it's desired
* to remove the `!` and properly fix the types on a case by case basis.
* These eslint below are metrics in form of warnings on how these are progressing.
* The initial warning amount is 297 warnings, slowly this number should be decreasing,
* as usage of `!` is not recommended and an escape hatch we needed in order to enable
* strict typing on all the other areas of the codebase.
*
*/
"@typescript-eslint/no-non-null-assertion": "warn",
"@typescript-eslint/no-non-null-asserted-optional-chain": "warn"
}
}
53 changes: 23 additions & 30 deletions src/components/axis.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { axisRight, axisBottom } from 'd3-axis';
import { BaseType, Selection } from 'd3-selection';
import { Selection } from 'd3-selection';
import { ScaleLinear, scaleLinear } from 'd3-scale';
import { OnResizeEvent, OnRescaleEvent } from '../interfaces';

Expand All @@ -10,27 +10,20 @@ export type Options = {
};

export class Axis {
private mainGroup: Selection<SVGElement, unknown, null, undefined>;
private mainGroup: Selection<SVGSVGElement, unknown, null, undefined>;
private _scaleX: ScaleLinear<number, number>;
private _scaleY: ScaleLinear<number, number>;
private _showLabels = true;
private _labelXDesc: string;
private _labelYDesc: string;
private _unitOfMeasure: string;
private _offsetX: number = 0;
private _offsetY: number = 0;
private _offsetX = 0;
private _offsetY = 0;
private _flipX = false;
private _flipY = false;
private visible: boolean = true;

constructor(
mainGroup: Selection<SVGElement, unknown, null, undefined>,
showLabels = true,
labelXDesc: string,
labelYDesc: string,
unitOfMeasure: string,
options?: Options,
) {
private visible = true;

constructor(mainGroup: Axis['mainGroup'], showLabels = true, labelXDesc: string, labelYDesc: string, unitOfMeasure: string, options?: Options) {
this.mainGroup = mainGroup;
this._showLabels = showLabels;
this._labelXDesc = labelXDesc;
Expand All @@ -51,12 +44,12 @@ export class Axis {
this._scaleY = scaleLinear().domain([0, 1]).range([0, 1]);
}

private renderLabelx(): Selection<BaseType, unknown, null, undefined> {
private renderLabelx(): Selection<SVGTextElement, unknown, null, undefined> {
const { _labelXDesc: labelXDesc, _unitOfMeasure: unitOfMeasure, _showLabels, _scaleX: scaleX } = this;
const [, width] = scaleX.range();
const gx = this.renderGx();

let labelx = gx.select('text.axis-labelx');
let labelx: Selection<SVGTextElement, unknown, null, undefined> = gx.select('text.axis-labelx');
if (_showLabels) {
if (labelx.empty()) {
labelx = gx
Expand All @@ -71,16 +64,16 @@ export class Axis {
} else {
labelx.remove();
}
labelx.attr('transform', `translate(${width / 2},-4)`);
labelx.attr('transform', `translate(${width! / 2},-4)`);
return labelx;
}

private renderLabely(): Selection<BaseType, unknown, null, undefined> {
private renderLabely(): Selection<SVGTextElement, unknown, null, undefined> {
const { _labelYDesc: labelYDesc, _unitOfMeasure: unitOfMeasure, _showLabels, _scaleY } = this;
const [, height] = _scaleY.range();
const gy = this.renderGy();

let labely = gy.select('text.axis-labely');
let labely: Selection<SVGTextElement, unknown, null, undefined> = gy.select('text.axis-labely');
if (_showLabels) {
if (labely.empty()) {
labely = gy
Expand All @@ -92,16 +85,16 @@ export class Axis {
.style('font-size', '10px')
.text(`${labelYDesc} (${unitOfMeasure})`);
}
labely.attr('transform', `translate(-10,${height / 2})rotate(90)`);
labely.attr('transform', `translate(-10,${height! / 2})rotate(90)`);
} else {
labely.remove();
}
return labely;
}

private renderGy(): Selection<BaseType, unknown, null, undefined> {
private renderGy(): Selection<SVGGElement, unknown, null, undefined> {
const { _scaleX, _scaleY } = this;
const yAxis = axisRight(_scaleY) as (selection: Selection<SVGSVGElement, unknown, null, undefined>) => void;
const yAxis = axisRight(_scaleY) as (selection: Selection<SVGGElement, unknown, null, undefined>, ...args: any[]) => void;
const [, width] = _scaleX.range();
const gy = this.createOrGet('y-axis');
gy.call(yAxis);
Expand All @@ -110,9 +103,9 @@ export class Axis {
return gy;
}

private renderGx(): Selection<BaseType, unknown, null, undefined> {
private renderGx(): Selection<SVGGElement, unknown, null, undefined> {
const { _scaleX, _scaleY } = this;
const xAxis = axisBottom(_scaleX) as (selection: Selection<SVGSVGElement, unknown, null, undefined>) => void;
const xAxis = axisBottom(_scaleX) as (selection: Selection<SVGGElement, unknown, null, undefined>, ...args: any[]) => void;
const [, height] = _scaleY.range();

const gx = this.createOrGet('x-axis');
Expand All @@ -121,9 +114,9 @@ export class Axis {
return gx;
}

private createOrGet = (name: string): Selection<BaseType, unknown, null, undefined> => {
private createOrGet = (name: string): Selection<SVGGElement, unknown, null, undefined> => {
const { mainGroup } = this;
let res = mainGroup.select(`g.${name}`);
let res: Selection<SVGGElement, unknown, null, undefined> = mainGroup.select(`g.${name}`);
if (res.empty()) {
res = mainGroup.append('g').attr('class', name);
}
Expand All @@ -142,8 +135,8 @@ export class Axis {
onRescale(event: OnRescaleEvent): void {
const { _scaleX, _scaleY, offsetX, offsetY } = this;
const { xScale, yScale } = event;
const xBounds = xScale.domain();
const yBounds = yScale.domain();
const xBounds = xScale.domain() as [number, number];
const yBounds = yScale.domain() as [number, number];

const xRange = xScale.range();
const yRange = yScale.range();
Expand Down Expand Up @@ -173,15 +166,15 @@ export class Axis {

flipX(flipX: boolean): Axis {
this._flipX = flipX;
const domain = this._scaleX.domain();
const domain = this._scaleX.domain() as [number, number];
const flip = flipX ? -1 : 1;
this._scaleX.domain([flip * domain[0], flip * domain[1]]);
return this;
}

flipY(flipY: boolean): Axis {
this._flipY = flipY;
const domain = this._scaleY.domain();
const domain = this._scaleY.domain() as [number, number];
const flip = flipY ? -1 : 1;
this._scaleY.domain([flip * domain[0], flip * domain[1]]);
return this;
Expand Down
14 changes: 7 additions & 7 deletions src/control/ExtendedCurveInterpolator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ export class ExtendedCurveInterpolator extends CurveInterpolator {
this.generateArcLengthLookup();
}
const index = BinarySearch.search(this.arcLengthLookup, arcLength);
const v1 = this.arcLengthLookup[index];
const v2 = this.arcLengthLookup[index + 1];
const v1 = this.arcLengthLookup[index]!;
const v2 = this.arcLengthLookup[index + 1]!;
const t = (index + (arcLength - v1) / (v2 - v1)) / this.arcLengthLookup.length;
return t;
}

generateArcLengthLookup(segments: number = 1000): void {
generateArcLengthLookup(segments = 1000): void {
let lastPos = this.getPointAt(0);
let length = 0;
for (let i = 0; i < segments; i++) {
Expand Down Expand Up @@ -123,15 +123,15 @@ export class ExtendedCurveInterpolator extends CurveInterpolator {

if (from !== 0) {
const fromIndex = Math.floor(from * this.arcLengthLookup.length);
const fromLl = this.arcLengthLookup[fromIndex];
const fromLh = this.arcLengthLookup[fromIndex + 1];
const fromLl = this.arcLengthLookup[fromIndex]!;
const fromLh = this.arcLengthLookup[fromIndex + 1]!;
fromLength = fromLl + ((from * this.arcLengthLookup.length) % this.arcLengthLookup.length) * (fromLh - fromLl);
}

if (to !== 1) {
const toIndex = Math.floor(to * this.arcLengthLookup.length);
const toLl = this.arcLengthLookup[toIndex];
const toLh = this.arcLengthLookup[toIndex + 1];
const toLl = this.arcLengthLookup[toIndex]!;
const toLh = this.arcLengthLookup[toIndex + 1]!;
toLength = toLl + ((from * this.arcLengthLookup.length) % this.arcLengthLookup.length) * (toLh - toLl);
}

Expand Down
Loading

0 comments on commit ef0590b

Please sign in to comment.