PR feedback

Author: imolorhe

DEV.md

@@ -181,3 +181,5 @@ https://www.kickstarter.com/projects/curtisclow/the-wild-cosmos-1-3-a-sci-fi-fan
 - Call service directly instead of using effects?
 - Use memoized `createSelector` functions for selectors
 - Debug with redux devtools to see what actions are being dispatched and how long they take
+- Add better trackBy functions for `*ngFor` directives
+- memoize expensive functions

packages/altair-app/src/app/modules/altair/altair.module.ts

@@ -159,9 +159,7 @@ const providers = [
       WorkspaceEffects,
       ElectronEffects,
     ]),
-    StoreDevtoolsModule.instrument({
-      logOnly: environment.production,
-    }),
+    !environment.production ? StoreDevtoolsModule.instrument() : [],
     TranslateModule.forRoot({
       loader: {
         provide: TranslateLoader,

packages/altair-app/src/app/modules/altair/components/query-editor/query-editor.component.ts

@@ -188,11 +188,9 @@ export class QueryEditorComponent implements OnInit, AfterViewInit, OnChanges {
       this.updateNewEditorDisableLineNumber(this.disableLineNumbers);
     }
 
-    if (changes?.query?.currentValue) {
+    if (changes?.query?.currentValue && this.selectedIndex !== 0) {
       // Set current tab to Query if query is updated
-      if (this.selectedIndex !== 0) {
-        this.selectedIndex = 0;
-      }
+      this.selectedIndex = 0;
     }
 
     if (changes?.shortcutMapping?.currentValue) {

packages/altair-app/src/app/modules/altair/components/query-result/query-result.component.html

@@ -133,7 +133,7 @@
     >
       <div class="request-script-logs">
         <div class="request-script-logs__list">
-          @for (item of requestScriptLogs; track item.time) {
+          @for (item of requestScriptLogs; track item.id) {
             <div class="request-script-logs__line">
               <div class="request-script-logs__line-time">
                 {{ item.time | date: 'mediumTime' }}

packages/altair-app/src/app/modules/altair/containers/window/window.component.ts

@@ -2,6 +2,7 @@ import {
   catchError,
   distinctUntilChanged,
   map,
+  shareReplay,
   switchMap,
   take,
   withLatestFrom,
@@ -187,15 +188,16 @@ export class WindowComponent implements OnInit {
       select((state) => state.windowsMeta.activeWindowId)
     );
 
-    this.windowState$ = this.store.pipe(
-      withLatestFrom(this.windowId$),
+    this.windowState$ = this.windowId$.pipe(
       distinctUntilChanged(),
-      map(([state, windowId]) => {
-        return (
-          fromRoot.selectWindowState(windowId)(state) ??
-          fromRoot.getInitialPerWindowState()
-        );
-      })
+      switchMap((windowId) =>
+        this.store.pipe(
+          select(fromRoot.selectWindowState(windowId)),
+          distinctUntilChanged(),
+          map((state) => state ?? fromRoot.getInitialPerWindowState())
+        )
+      ),
+      shareReplay(1)
     );
 
     this.query$ = this.windowState$.pipe(select(fromRoot.getQueryState));

packages/altair-app/src/app/modules/altair/directives/cached-if.directive.spec.ts

@@ -1,8 +1,16 @@
+import { MockInstance } from 'ng-mocks';
 import { CachedIfDirective } from './cached-if.directive';
+import { TemplateRef, ViewContainerRef } from '@angular/core';
+import { mock } from 'testing';
 
 describe('CachedIfDirective', () => {
   it('should create an instance', () => {
-    const directive = new CachedIfDirective();
+    const directive = new CachedIfDirective(
+      // MockInstance(TemplateRef),
+      // MockInstance(ViewContainerRef),
+      mock(),
+      mock()
+    );
     expect(directive).toBeTruthy();
   });
 });

packages/altair-app/src/app/modules/altair/directives/cached-if.directive.ts

@@ -2,14 +2,15 @@ import {
   Directive,
   EmbeddedViewRef,
   Input,
+  OnDestroy,
   TemplateRef,
   ViewContainerRef,
 } from '@angular/core';
 
 @Directive({
   selector: '[appCachedIf]',
 })
-export class CachedIfDirective {
+export class CachedIfDirective implements OnDestroy {
   private hasView = false;
   private cachedViewRef?: EmbeddedViewRef<unknown>;
 
@@ -40,4 +41,10 @@ export class CachedIfDirective {
     }
     this.hasView = val;
   }
+  ngOnDestroy(): void {
+    if (this.cachedViewRef) {
+      this.cachedViewRef.destroy();
+      this.cachedViewRef = undefined;
+    }
+  }
 }

packages/altair-app/src/app/modules/altair/effects/query.effect.ts

@@ -346,10 +346,8 @@ export class QueryEffects {
                       new queryActions.AppendRequestScriptLogsAction(
                         response.windowId,
                         [
-                          ...new Set([
-                            ...(preRequestScriptLogs || []),
-                            ...(result?.transformedData?.requestScriptLogs || []),
-                          ]),
+                          ...(preRequestScriptLogs || []),
+                          ...(result?.transformedData?.requestScriptLogs || []),
                         ]
                       )
                     );

packages/altair-core/jest.config.js

@@ -9,4 +9,7 @@ module.exports = {
   },
   coverageDirectory: '<rootDir>/.coverage',
   collectCoverage: true,
+  transformIgnorePatterns: [
+    'node_modules/.pnpm/(?!@angular|@firebase|@ngrx|@sentry|lodash-es|altair-graphql-core|angular-resizable-element|angular-split|dexie|uuid|ngx-cookie-service|ngx-markdown|ky|color-name|json-schema-library|graphql-language-service|vscode-languageserver-types|cm6-graphql|ngx-toastr|@ngx-translate|lucide-angular|ng-zorro-antd|@ant-design\\+icons-angular|ngx-pipes)',
+  ],
 };