build: fix private/protected naming convention & improve boolean one (#…

…891) * build: fix naming convention rule for private/protected * build: repeat private rule using map * style: enforce more boolean naming conventions add comments * test: improve 404 page testing to avoid reading protected var
davidlj95 · Nov 29, 2024 · b6bec0d · b6bec0d
1 parent 4829a32
commit b6bec0d
Show file tree

Hide file tree

Showing 34 changed files with 102 additions and 96 deletions.
diff --git a/data/templates/environment.pull-request.ts.liquid b/data/templates/environment.pull-request.ts.liquid
@@ -9,6 +9,6 @@ const CLOUDFLARE_PAGES_PREVIEW_BRANCH_ALIAS_URL = new URL(
   `https://${PR_BRANCH_NAME}.${CLOUDFLARE_PAGES_DOMAIN}`,
 )
 export const environment: Environment = {
-  production: false,
+  isProduction: false,
   appBaseUrl: CLOUDFLARE_PAGES_PREVIEW_BRANCH_ALIAS_URL,
 }
diff --git a/eslint.config.mjs b/eslint.config.mjs
@@ -34,12 +34,14 @@ export const NAMING_CONVENTION_SELECTORS = [
     format: ['camelCase', 'UPPER_CASE'],
   },
   // https://github.com/typescript-eslint/typescript-eslint/blob/v8.16.0/packages/eslint-plugin/docs/rules/naming-convention.mdx#enforce-that-private-members-are-prefixed-with-an-underscore
-  {
+  // 👇 `modifiers` matches code with ALL modifiers specified
+  //    To specify the same for several modifiers, repeating the rule for each one
+  ...['private', 'protected'].map((modifier) => ({
     selector: 'memberLike',
-    modifiers: ['private', 'protected'],
+    modifiers: [modifier],
     format: ['camelCase'],
     leadingUnderscore: 'require',
-  },
+  })),
   // https://github.com/typescript-eslint/typescript-eslint/blob/v8.16.0/packages/eslint-plugin/docs/rules/naming-convention.mdx#enforce-that-interface-names-do-not-begin-with-an-i
   {
     selector: 'typeLike',

diff --git a/eslint.config.typed.mjs b/eslint.config.typed.mjs
@@ -23,12 +23,19 @@ export default tsEslint.config(
         // https://github.com/typescript-eslint/typescript-eslint/blob/v8.16.0/packages/eslint-plugin/docs/rules/naming-convention.mdx#enforce-that-boolean-variables-are-prefixed-with-an-allowed-verb
         {
           selector: [
-            'variable',
+            // `member-like` without:
+            //  - `property`: will be specified later, with exceptions
+            //  - `enumMember`: no types allowed
+            //  - `method`: no types allowed
             'classicAccessor',
             'autoAccessor',
+            'parameterProperty',
+            // All `property` but `objectLiteralProperty`
             'classProperty',
+            'typeProperty',
+            // The rest
             'parameter',
-            'parameterProperty',
+            'variable',
           ],
           types: ['boolean'],
           format: ['PascalCase'],

diff --git a/src/app/common/material-symbol.directive.ts b/src/app/common/material-symbol.directive.ts
@@ -4,10 +4,8 @@ import { Directive, ElementRef } from '@angular/core'
   selector: '[appMaterialSymbol]',
 })
 export class MaterialSymbolDirective {
-  constructor(private el: ElementRef) {
-    ;(this.el.nativeElement as HTMLElement).classList.add(
-      MATERIAL_SYMBOLS_CLASS,
-    )
+  constructor(elRef: ElementRef<Element>) {
+    elRef.nativeElement.classList.add(MATERIAL_SYMBOLS_CLASS)
   }
 }
 

diff --git a/src/app/common/test-id.directive.ts b/src/app/common/test-id.directive.ts
@@ -4,13 +4,10 @@ import { Directive, effect, ElementRef, input } from '@angular/core'
 export class TestIdDirective {
   readonly appTestId = input.required<string>()
 
-  constructor(private el: ElementRef) {
+  constructor(elRef: ElementRef<Element>) {
     effect(() => {
       if (isDevMode) {
-        ;(this.el.nativeElement as Element).setAttribute(
-          TEST_ID_ATTRIBUTE,
-          this.appTestId(),
-        )
+        elRef.nativeElement.setAttribute(TEST_ID_ATTRIBUTE, this.appTestId())
       }
     })
   }

diff --git a/src/app/header/light-dark-toggle/color-scheme.service.spec.ts b/src/app/header/light-dark-toggle/color-scheme.service.spec.ts
@@ -245,17 +245,17 @@ class MockHTMLElementWithAttributes
   implements
     Pick<HTMLElement, 'getAttribute' | 'setAttribute' | 'removeAttribute'>
 {
-  private attributes = new Map<string, string>()
+  private readonly _attributes = new Map<string, string>()
 
   getAttribute(qualifiedName: string) {
-    return this.attributes.get(qualifiedName) ?? null
+    return this._attributes.get(qualifiedName) ?? null
   }
 
   setAttribute(qualifiedName: string, value: string) {
-    return this.attributes.set(qualifiedName, value)
+    return this._attributes.set(qualifiedName, value)
   }
 
   removeAttribute(qualifiedName: string): void {
-    this.attributes.delete(qualifiedName)
+    this._attributes.delete(qualifiedName)
   }
 }
diff --git a/src/app/header/light-dark-toggle/color-scheme.service.ts b/src/app/header/light-dark-toggle/color-scheme.service.ts
@@ -20,14 +20,14 @@ export class ColorSchemeService {
     @Inject(WINDOW) private _window: Window,
   ) {
     this._htmlElement = document.documentElement
-    this.listenForMatchMediaPreferenceChanges()
+    this._listenForMatchMediaPreferenceChanges()
   }
 
-  private get isDarkPreferred(): boolean {
-    return !!this.matchMediaQuery && this.matchMediaQuery.matches
+  private get _isDarkPreferred(): boolean {
+    return !!this._matchMediaQuery && this._matchMediaQuery.matches
   }
 
-  private get matchMediaQuery() {
+  private get _matchMediaQuery() {
     if (!this._window.matchMedia) {
       return null
     }
@@ -39,7 +39,7 @@ export class ColorSchemeService {
       HTML_COLOR_SCHEME_ATTRIBUTE,
     ) as Scheme
     if (!manuallySetScheme) {
-      this.setManual(this.isDarkPreferred ? Scheme.Light : Scheme.Dark)
+      this.setManual(this._isDarkPreferred ? Scheme.Light : Scheme.Dark)
       return
     }
 
@@ -56,8 +56,8 @@ export class ColorSchemeService {
     this._htmlElement.removeAttribute(HTML_COLOR_SCHEME_ATTRIBUTE)
   }
 
-  private listenForMatchMediaPreferenceChanges() {
-    this.matchMediaQuery?.addEventListener('change', () => {
+  private _listenForMatchMediaPreferenceChanges() {
+    this._matchMediaQuery?.addEventListener('change', () => {
       this.setSystem()
     })
   }

diff --git a/src/app/header/light-dark-toggle/light-dark-toggle.component.html b/src/app/header/light-dark-toggle/light-dark-toggle.component.html
@@ -1,14 +1,14 @@
 <button
   app-toolbar-button
-  [icon]="MaterialSymbol.DarkMode"
-  (click)="colorSchemeService.toggleDarkLight()"
+  [icon]="_materialSymbol.DarkMode"
+  (click)="_colorSchemeService.toggleDarkLight()"
   aria-label="Switch to dark mode"
   class="light-only"
 ></button>
 <button
   app-toolbar-button
-  [icon]="MaterialSymbol.LightMode"
-  (click)="colorSchemeService.toggleDarkLight()"
+  [icon]="_materialSymbol.LightMode"
+  (click)="_colorSchemeService.toggleDarkLight()"
   aria-label="Switch to light mode"
   class="dark-only"
 ></button>
diff --git a/src/app/header/light-dark-toggle/light-dark-toggle.component.ts b/src/app/header/light-dark-toggle/light-dark-toggle.component.ts
@@ -10,10 +10,10 @@ import { ToolbarButtonComponent } from '../toolbar-button/toolbar-button.compone
   styleUrl: './light-dark-toggle.component.scss',
 })
 export class LightDarkToggleComponent {
-  protected readonly MaterialSymbol = {
+  protected readonly _materialSymbol = {
     DarkMode,
     LightMode,
   }
 
-  constructor(protected readonly colorSchemeService: ColorSchemeService) {}
+  constructor(protected readonly _colorSchemeService: ColorSchemeService) {}
 }
diff --git a/src/app/header/tabs/tabs.component.html b/src/app/header/tabs/tabs.component.html
@@ -1,6 +1,6 @@
 <button
   app-toolbar-button
-  [icon]="MaterialSymbol.KeyboardDoubleArrowLeft"
+  [icon]="_materialSymbol.KeyboardDoubleArrowLeft"
   aria-label="Previous tab"
   [disabled]="_prevButtonDisabled()"
   (click)="_scrollABit(-1)"
@@ -11,7 +11,7 @@
 </div>
 <button
   app-toolbar-button
-  [icon]="MaterialSymbol.KeyboardDoubleArrowRight"
+  [icon]="_materialSymbol.KeyboardDoubleArrowRight"
   aria-label="Next tab"
   [disabled]="_nextButtonDisabled()"
   (click)="_scrollABit(1)"

diff --git a/src/app/header/tabs/tabs.component.ts b/src/app/header/tabs/tabs.component.ts
@@ -27,7 +27,7 @@ import { TabComponent } from '../tab/tab.component'
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
 export class TabsComponent implements OnDestroy {
-  protected readonly MaterialSymbol = {
+  protected readonly _materialSymbol = {
     KeyboardDoubleArrowLeft,
     KeyboardDoubleArrowRight,
   }

diff --git a/src/app/no-script/no-script.component.html b/src/app/no-script/no-script.component.html
@@ -1,7 +1,7 @@
 <noscript>
   <div class="contents">
     <p>
-      <span appMaterialSymbol>{{ MaterialSymbol.Warning }}</span>
+      <span appMaterialSymbol>{{ _materialSymbol.Warning }}</span>
       <b>Seems you don't have JavaScript enabled</b>
     </p>
     <p>

diff --git a/src/app/no-script/no-script.component.ts b/src/app/no-script/no-script.component.ts
@@ -10,7 +10,7 @@ import { MaterialSymbolDirective } from '@/common/material-symbol.directive'
   host: { ngSkipHydration: 'true' },
 })
 export class NoScriptComponent {
-  protected readonly MaterialSymbol = {
+  protected readonly _materialSymbol = {
     Warning,
   }
 }
diff --git a/src/app/not-found-page/not-found-page.component.html b/src/app/not-found-page/not-found-page.component.html
@@ -15,14 +15,14 @@ <h1>
 </main>
 <aside class="tip">
   <header>
-    <span appMaterialSymbol>{{ MaterialSymbol.Lightbulb }}</span
+    <span appMaterialSymbol>{{ _materialSymbol.Lightbulb }}</span
     ><b>TIP: What if this page existed before?</b>
   </header>
   <p>
     You can use
     <a href="https://web.archive.org/">Internet Archive's Wayback Machine</a>
     to see if a page existed in the past. <br />Maybe
-    <a [href]="currentUrlInWaybackMachine"
+    <a [href]="_currentUrlInWaybackMachine" appTestId="wayback"
       >this page existed at some other point in time</a
     >
   </p>

diff --git a/src/app/not-found-page/not-found-page.component.spec.ts b/src/app/not-found-page/not-found-page.component.spec.ts
@@ -8,20 +8,21 @@ import { Router } from '@angular/router'
 import { MockProvider } from 'ng-mocks'
 import { APP_BASE_URL } from '@/common/app-base-url'
 import { componentTestSetup } from '@/test/helpers/component-test-setup'
+import { byTestId } from '@/test/helpers/test-id'
 
 describe('NotFoundPageComponent', () => {
   let component: NotFoundPageComponent
   let fixture: ComponentFixture<NotFoundPageComponent>
-  const dummyAppUrlNoTrailingSlash = 'https://example.com'
-  const dummyRouter: Pick<Router, 'url'> = {
+  const DUMMY_APP_URL = 'https://example.com'
+  const DUMMY_ROUTER: Pick<Router, 'url'> = {
     url: '/foo',
   }
 
   beforeEach(() => {
     ;[fixture, component] = componentTestSetup(NotFoundPageComponent, {
       providers: [
-        MockProvider(APP_BASE_URL, new URL(dummyAppUrlNoTrailingSlash)),
-        MockProvider(Router, dummyRouter),
+        MockProvider(APP_BASE_URL, new URL(DUMMY_APP_URL)),
+        MockProvider(Router, DUMMY_ROUTER),
       ],
     })
     fixture.detectChanges()
@@ -31,15 +32,15 @@ describe('NotFoundPageComponent', () => {
     expect(component).toBeTruthy()
   })
 
-  describe('#currentUrlInWaybackMachine', () => {
-    it('should be the Wayback Machine URL prefix plus the current URL', () => {
-      expect(component.currentUrlInWaybackMachine).toEqual(
-        new URL(
-          WAYBACK_MACHINE_URL_PREFIX.toString() +
-            dummyAppUrlNoTrailingSlash +
-            dummyRouter.url,
-        ),
-      )
-    })
+  it('should contain a link to the Wayback Machine URL of the current page', () => {
+    const anchorElement = fixture.debugElement.query(byTestId('wayback'))
+
+    expect(anchorElement.attributes['href']).toEqual(
+      new URL(
+        WAYBACK_MACHINE_URL_PREFIX.toString() +
+          DUMMY_APP_URL +
+          DUMMY_ROUTER.url,
+      ).toString(),
+    )
   })
 })
diff --git a/src/app/not-found-page/not-found-page.component.ts b/src/app/not-found-page/not-found-page.component.ts
@@ -3,23 +3,23 @@ import { Lightbulb } from '@/data/material-symbols'
 import { Router } from '@angular/router'
 import { MaterialSymbolDirective } from '@/common/material-symbol.directive'
 import { APP_BASE_URL } from '@/common/app-base-url'
+import { TestIdDirective } from '@/common/test-id.directive'
 
 @Component({
   selector: 'app-not-found-page',
   templateUrl: './not-found-page.component.html',
   styleUrls: ['./not-found-page.component.scss'],
-  imports: [MaterialSymbolDirective],
+  imports: [MaterialSymbolDirective, TestIdDirective],
 })
 export class NotFoundPageComponent {
-  readonly currentUrlInWaybackMachine: URL
-  protected readonly MaterialSymbol = {
+  protected readonly _currentUrlInWaybackMachine: URL
+  protected readonly _materialSymbol = {
     Lightbulb,
   }
 
   constructor(@Inject(APP_BASE_URL) appBaseUrl: URL, router: Router) {
-    const currentUrl = new URL(router.url, appBaseUrl)
-    this.currentUrlInWaybackMachine = new URL(
-      `${WAYBACK_MACHINE_URL_PREFIX}${currentUrl}`,
+    this._currentUrlInWaybackMachine = new URL(
+      `${WAYBACK_MACHINE_URL_PREFIX}${new URL(router.url, appBaseUrl)}`,
     )
   }
 }

diff --git a/src/app/resume-page/chip/chip.component.ts b/src/app/resume-page/chip/chip.component.ts
@@ -9,9 +9,9 @@ import { Component, EventEmitter, input, Output } from '@angular/core'
     '[class.selectable]': 'isSelectedChange.observed',
     '[attr.role]': "isSelectedChange.observed ? 'button': undefined",
     '[attr.tabindex]': 'isSelectedChange.observed ? 0 : undefined',
-    '(click)': 'emitToggledSelected()',
-    '(keydown.enter)': 'emitToggledSelected()',
-    '(keydown.space)': 'emitToggledSelected()',
+    '(click)': '_emitToggledSelected()',
+    '(keydown.enter)': '_emitToggledSelected()',
+    '(keydown.space)': '_emitToggledSelected()',
   },
 })
 export class ChipComponent {
@@ -20,7 +20,7 @@ export class ChipComponent {
   @Output()
   isSelectedChange = new EventEmitter<boolean>()
 
-  protected emitToggledSelected() {
+  protected _emitToggledSelected() {
     this.isSelectedChange.emit(!this.isSelected())
   }
 }
diff --git a/src/app/resume-page/collapsible-tree/collapsible-tree.component.ts b/src/app/resume-page/collapsible-tree/collapsible-tree.component.ts
@@ -55,11 +55,11 @@ export class CollapsibleTreeComponent {
   isExpanded = false
 
   @ViewChildren(CollapsibleTreeComponent)
-  private children!: QueryList<CollapsibleTreeComponent>
+  private readonly _children!: QueryList<CollapsibleTreeComponent>
 
   private readonly _id = nextId++
 
-  constructor(private readonly cdRef: ChangeDetectorRef) {}
+  constructor(private readonly _cdRef: ChangeDetectorRef) {}
 
   get isCollapsible(): boolean {
     if (!(this.node.children.length > 0)) {
@@ -81,7 +81,7 @@ export class CollapsibleTreeComponent {
   collapse() {
     if (this.isExpanded) {
       this.isExpanded = false
-      this.cdRef.markForCheck()
+      this._cdRef.markForCheck()
     }
   }
 
@@ -93,7 +93,9 @@ export class CollapsibleTreeComponent {
   }
 
   collapseAllChildren({ except }: { except?: CollapsibleTreeComponent } = {}) {
-    const childrenToCollapse = this.children.filter((child) => child !== except)
+    const childrenToCollapse = this._children.filter(
+      (child) => child !== except,
+    )
     for (const child of childrenToCollapse) {
       child.collapse()
     }

diff --git a/src/app/resume-page/education-section/education-item/education-item.component.html b/src/app/resume-page/education-section/education-item/education-item.component.html
@@ -25,7 +25,7 @@
     <app-card-header-attributes>
       @if (item.isCumLaude) {
         <app-attribute
-          [symbol]="MaterialSymbol.SocialLeaderboard"
+          [symbol]="_materialSymbol.SocialLeaderboard"
           [appTestId]="_attribute.CumLaude"
         >
           Cum laude

diff --git a/src/app/resume-page/education-section/education-item/education-item.component.ts b/src/app/resume-page/education-section/education-item/education-item.component.ts
@@ -46,7 +46,7 @@ export class EducationItemComponent {
     return name.length > 15 && shortName ? shortName : name
   })
 
-  protected readonly MaterialSymbol = {
+  protected readonly _materialSymbol = {
     SocialLeaderboard,
   }
   protected readonly _attribute = ATTRIBUTE

diff --git a/src/app/resume-page/education-section/education-section.component.html b/src/app/resume-page/education-section/education-section.component.html
@@ -1,6 +1,6 @@
 <app-section-title><h2 id="education">Education</h2></app-section-title>
 <app-card-grid>
-  @for (item of items; track item) {
+  @for (item of _items; track item) {
     <app-education-item [item]="item"></app-education-item>
   }
 </app-card-grid>