refactor: optimizes the handling of extension settings and adapts them to the JavaScript review standard for GNOME extensions

Author: leandromqrs

esbuild.ts

@@ -18,6 +18,10 @@ interface ExtensionMetadata {
   uuid: string;
 }
 
+const currentDir = dirname(fileURLToPath(import.meta.url));
+const metadataPath = resolve(currentDir, 'metadata.json');
+const metadata = JSON.parse(readFileSync(metadataPath, 'utf8')) as ExtensionMetadata;
+
 // Externalize local .ts imports and rewrite paths to .js
 const localExternals: Plugin = {
   name: 'local-externals',
@@ -34,9 +38,137 @@ const localExternals: Plugin = {
   },
 };
 
-const currentDir = dirname(fileURLToPath(import.meta.url));
-const metadataPath = resolve(currentDir, 'metadata.json');
-const metadata = JSON.parse(readFileSync(metadataPath, 'utf8')) as ExtensionMetadata;
+// Resolve @girs/* imports directly to gi:// and resource:// runtime paths,
+// bypassing node_modules to avoid re-export shims and version query strings
+const girsResolver: Plugin = {
+  name: 'girs-resolver',
+  setup(ctx) {
+    // @girs/gjs is a type-only shim; drop it entirely
+    ctx.onResolve({ filter: /^@girs\/gjs$/ }, () => ({
+      path: 'gjs',
+      namespace: 'girs-empty',
+    }));
+    ctx.onLoad({ filter: /.*/, namespace: 'girs-empty' }, () => ({
+      contents: '',
+      loader: 'js',
+    }));
+
+    // @girs/gnome-shell/* → resource:///org/gnome/shell/*.js
+    ctx.onResolve({ filter: /^@girs\/gnome-shell\// }, (args) => {
+      const subpath = args.path.replace('@girs/gnome-shell/', '');
+      const distFile = resolve(
+        currentDir,
+        'node_modules/@girs/gnome-shell/dist',
+        `${subpath}.js`,
+      );
+      // Read the actual resource path from the package when the file exists,
+      // otherwise fall back to the standard resource path pattern
+      if (existsSync(distFile)) {
+        const content = readFileSync(distFile, 'utf8');
+        const match = content.match(/from ['"](.+)['"]/);
+        if (match) return { path: match[1], external: true };
+      }
+      return {
+        path: `resource:///org/gnome/shell/${subpath}.js`,
+        external: true,
+      };
+    });
+
+    // @girs/<name>-<version> → gi://<Namespace> (without ?version=)
+    ctx.onResolve({ filter: /^@girs\// }, (args) => {
+      if (args.path === '@girs/gjs' || args.path.startsWith('@girs/gnome-shell/'))
+        return;
+      const pkgName = args.path.replace('@girs/', '');
+      const mainFile = resolve(
+        currentDir,
+        `node_modules/@girs/${pkgName}/${pkgName}.js`,
+      );
+      if (existsSync(mainFile)) {
+        const content = readFileSync(mainFile, 'utf8');
+        const match = content.match(/from ["'](gi:\/\/[^?"']+)/);
+        if (match) return { path: match[1], external: true };
+      }
+    });
+  },
+};
+
+// JS keywords that should NOT be treated as method declarations
+const JS_KEYWORDS = new Set([
+  'if', 'for', 'while', 'switch', 'return', 'throw', 'try', 'catch',
+  'finally', 'else', 'do', 'new', 'typeof', 'void', 'delete', 'await',
+  'yield', 'debugger', 'with', 'break', 'continue', 'case', 'default',
+  'super', 'this', 'true', 'false', 'null', 'undefined',
+]);
+
+// Add blank lines between class methods, between the last field and first
+// method, and between top-level declarations. esbuild strips these blank
+// lines during compilation and prettier does not re-add them.
+function addBlankLinesBetweenMembers(code: string): string {
+  const lines = code.split('\n');
+  const result: string[] = [];
+
+  const isMethodDecl = (trimmed: string): boolean => {
+    const m = trimmed.match(
+      /^(?:(?:get |set |static |async )*([a-zA-Z_$#]\w*)\s*\()/,
+    );
+    return m !== null && !JS_KEYWORDS.has(m[1]);
+  };
+
+  const isComment = (trimmed: string): boolean =>
+    trimmed.startsWith('//') || trimmed.startsWith('/*') || trimmed.startsWith('*');
+
+  for (let i = 0; i < lines.length; i++) {
+    if (i > 0 && lines[i].trim() !== '' && lines[i - 1].trim() !== '') {
+      const prev = lines[i - 1];
+      const curr = lines[i];
+      const prevTrimmed = prev.trimStart();
+      const currTrimmed = curr.trimStart();
+      const prevIndent = prev.search(/\S|$/);
+      const currIndent = curr.search(/\S|$/);
+      let needsBlank = false;
+
+      const isClosingBrace = /^\};?$/.test(prevTrimmed);
+
+      // Between methods: closing } at indent N → method decl at indent N
+      if (
+        isClosingBrace &&
+        currIndent === prevIndent &&
+        (isMethodDecl(currTrimmed) || isComment(currTrimmed))
+      ) {
+        needsBlank = true;
+      }
+
+      // Between last field and first method at the same indent level
+      if (
+        !needsBlank &&
+        /;$/.test(prevTrimmed) &&
+        /^[a-zA-Z_$#]\w*/.test(prevTrimmed) &&
+        currIndent === prevIndent &&
+        isMethodDecl(currTrimmed)
+      ) {
+        needsBlank = true;
+      }
+
+      // Between top-level declarations (indent 0) after block closures
+      if (
+        !needsBlank &&
+        currIndent === 0 &&
+        /[})\]];?\s*$/.test(prevTrimmed) &&
+        /^(?:var |let |const |function |class |export )/.test(currTrimmed)
+      ) {
+        needsBlank = true;
+      }
+
+      if (needsBlank) {
+        result.push('');
+      }
+    }
+
+    result.push(lines[i]);
+  }
+
+  return result.join('\n');
+}
 
 const srcDir = resolve(currentDir, 'src');
 const entryPoints = (readdirSync(srcDir, { recursive: true }) as string[])
@@ -49,7 +181,7 @@ const sharedOptions = {
   outdir: 'dist',
   outbase: 'src',
   bundle: true,
-  plugins: [localExternals],
+  plugins: [localExternals, girsResolver],
   // Do not remove the functions `enable()`, `disable()` and `init()`
   treeShaking: false,
   // firefox60  // Since GJS 1.53.90
@@ -59,10 +191,6 @@ const sharedOptions = {
   // firefox102 // Since GJS 1.73.2
   target: 'firefox102' as const,
   format: 'esm' as const,
-  alias: {
-    // Not exported in @girs/gnome-shell v49; map directly to runtime resource
-    '@girs/gnome-shell/ui/dash': 'resource:///org/gnome/shell/ui/dash.js',
-  },
   external: ['gi://*', 'resource://*', 'system', 'gettext', 'cairo'],
 };
 
@@ -91,7 +219,7 @@ Promise.all(
       const filePath = resolve(distDir, file);
       const content = readFileSync(filePath, 'utf8');
       const formatted = await format(content, { parser: 'babel', filepath: filePath });
-      writeFileSync(filePath, formatted);
+      writeFileSync(filePath, addBlankLinesBetweenMembers(formatted));
     }
 
     copyFileSync(metadataPath, metaDist);

src/extension.ts

@@ -28,7 +28,6 @@ const MODULE_FACTORIES: Record<string, () => Module> = {
 export default class AuroraShellExtension extends Extension {
   private _modules: Map<string, Module> = new Map();
   private _settings: Gio.Settings | null = null;
-  private _settingsHandlers: number[] = [];
 
   override enable(): void {
     console.log('Enabling extension');
@@ -60,12 +59,15 @@ export default class AuroraShellExtension extends Extension {
   private _connectSettings(): void {
     if (!this._settings) return;
 
+    const args: any[] = [];
     for (const def of MODULE_REGISTRY) {
-      const handlerId = this._settings.connect(`changed::${def.settingsKey}`, () => {
+      args.push(`changed::${def.settingsKey}`, () => {
         this._toggleModule(def);
       });
-      this._settingsHandlers.push(handlerId);
     }
+    args.push(this);
+
+    this._settings.connectObject(...args);
   }
 
   private _toggleModule(def: ModuleDefinition): void {
@@ -95,12 +97,7 @@ export default class AuroraShellExtension extends Extension {
   override disable(): void {
     console.log('Aurora Shell: Disabling extension');
 
-    if (this._settings) {
-      for (const handlerId of this._settingsHandlers) {
-        this._settings.disconnect(handlerId);
-      }
-      this._settingsHandlers = [];
-    }
+    this._settings?.disconnectObject(this);
 
     for (const [name, module] of this._modules) {
       try {

src/modules/dock.ts

@@ -44,7 +44,6 @@ type ManagedDockBinding = {
   intellihide: InstanceType<typeof DockIntellihide>;
   hotArea: InstanceType<typeof DockHotArea> | null;
   autoHideReleaseId: number;
-  destroyed: boolean;
   hotAreaActive: boolean;
 };
 
@@ -160,7 +159,6 @@ const DockIntellihide = GObject.registerClass({
   private _status: OverlapStatus = OverlapStatus.CLEAR;
   private _focusActor: any = null;
   private _focusActorId = 0;
-  private _destroyed = false;
 
   _init(monitorIndex: number) {
     super._init();
@@ -195,9 +193,6 @@ const DockIntellihide = GObject.registerClass({
   }
 
   override destroy(): void {
-    if (this._destroyed) return;
-    this._destroyed = true;
-
     this._disconnectFocusActor();
     global.display.disconnectObject(this);
     Main.layoutManager.disconnectObject(this);
@@ -206,8 +201,6 @@ const DockIntellihide = GObject.registerClass({
     Main.keyboard.disconnectObject(this);
     Main.overview.disconnectObject(this);
     this.disconnectObject?.(this);
-
-    (this as unknown as { run_dispose?: () => void }).run_dispose?.();
   }
 
   private _checkOverlap(): void {
@@ -388,7 +381,6 @@ export class Dock extends Module {
       intellihide,
       hotArea: null,
       autoHideReleaseId: 0,
-      destroyed: false,
       hotAreaActive: false,
     };
 
@@ -469,10 +461,11 @@ export class Dock extends Module {
   }
 
   private _destroyBinding(binding: ManagedDockBinding): void {
-    if (binding.destroyed) return;
-    binding.destroyed = true;
+    if (binding.autoHideReleaseId) {
+      GLib.source_remove(binding.autoHideReleaseId);
+      binding.autoHideReleaseId = 0;
+    }
 
-    this._clearHotAreaReveal(binding);
     binding.intellihide.disconnectObject?.(this);
     binding.hotArea?.disconnectObject?.(this);
 

src/modules/pipOnTop.ts

@@ -63,8 +63,6 @@ export class PipOnTop extends Module {
   }
 
   private _disconnectWorkspace(): void {
-    if (!this._lastWorkspace) return;
-
     if (this._windowAddedId) {
       this._lastWorkspace.disconnect(this._windowAddedId);
       this._windowAddedId = 0;

src/ui/dash.ts

@@ -40,8 +40,8 @@ interface AuroraDashParams {
  * - The **dash** (this widget) always sits at local y=0 inside the container.
  *   Show/hide animations use `translation_y` only — never modify `this.y`.
  */
-@GObject.registerClass
-export class AuroraDash extends Dash {
+export const AuroraDash = GObject.registerClass(
+class AuroraDashClass extends Dash {
   private declare _monitorIndex: number;
   private _workArea: DashBounds | null = null;
   private _container: St.Bin | null = null;
@@ -55,7 +55,6 @@ export class AuroraDash extends Dash {
   private _targetBoxListener: TargetBoxListener | null = null;
   private _pendingShow: { animate: boolean; onComplete?: () => void } | null = null;
   private _cycleState: { appId: string; windows: any[]; index: number } | null = null;
-  private _isDestroyed = false;
 
   _init(params: AuroraDashParams = {}): void {
     super._init();
@@ -115,9 +114,6 @@ export class AuroraDash extends Dash {
   }
 
   override destroy(): void {
-    if (this._isDestroyed) return;
-    this._isDestroyed = true;
-
     this._clearAllTimeouts();
 
     (this as any).showAppsButton?.disconnectObject?.(this);
@@ -137,7 +133,6 @@ export class AuroraDash extends Dash {
   }
 
   override _queueRedisplay(): void {
-    if (this._isDestroyed) return;
     super._queueRedisplay();
   }
 
@@ -651,7 +646,7 @@ export class AuroraDash extends Dash {
     if (this._workAreaUpdateId) return;
     this._workAreaUpdateId = GLib.idle_add(GLib.PRIORITY_DEFAULT_IDLE, () => {
       this._workAreaUpdateId = 0;
-      if (!this._isDestroyed && this._workArea) {
+      if (this._workArea) {
         this.applyWorkArea(this._workArea);
       }
       return GLib.SOURCE_REMOVE;
@@ -671,7 +666,9 @@ export class AuroraDash extends Dash {
     this._clearTimeout('_blockAutoHideDelayId');
     this._clearTimeout('_workAreaUpdateId');
   }
-}
+});
+
+export type AuroraDash = InstanceType<typeof AuroraDash>;
 
 function boundsEqual(a: DashBounds | null, b: DashBounds | null): boolean {
   if (a === b) return true;