fix: protect internal methods (#422)

Author: aklkv

src/flash/object.ts

@@ -35,6 +35,18 @@ export type FlashObjectOptions<
   onDidExitMessage?: () => void;
 } & T;
 
+const INTERNAL_KEYS = new Set([
+  // Lifecycle callbacks — already assigned to `this` above the loop, but
+  // listed here as a safety net in case future refactors change that order.
+  'onDestroy',
+  'onDidDestroyMessage',
+  'onDidExitMessage',
+  // Options consumed by the constructor but not stored as class properties.
+  'testHelperDisableTimeout',
+  // Stored in a private field (#flashService), so `key in this` won't catch it.
+  'flashService',
+]);
+
 export default class FlashObject<
   T extends Record<string, unknown> = Record<string, unknown>,
 > {
@@ -99,7 +111,7 @@ export default class FlashObject<
 
     // Copy any custom properties from T
     for (const [key, value] of Object.entries(options)) {
-      if (!(key in this) && key !== 'flashService' && !key.startsWith('on')) {
+      if (!(key in this) && !INTERNAL_KEYS.has(key)) {
         (this as Record<string, unknown>)[key] = value;
       }
     }

src/services/flash-messages.ts

@@ -214,6 +214,11 @@ export default class FlashMessagesService<
   ): boolean {
     const message = this.findBy(key, value);
     if (message) {
+      // Remove from queue immediately so the message disappears from the UI
+      // without waiting for the extendedTimeout exit-animation delay.
+      // destroyMessage() still handles timer cleanup and destroyable teardown.
+      this.queue = this.queue.filter((flash) => flash !== message);
+      this.#guidSet.delete(message._guid);
       message.destroyMessage();
       return true;
     }

tests/unit/flash/object-test.ts

@@ -344,4 +344,71 @@ module('Unit | Flash | object', function (hooks) {
 
     assert.notOk(flash.timerTaskInstance, 'no timer when timeout is undefined');
   });
+
+  test('custom properties starting with "on" are preserved', function (assert) {
+    interface CustomFields extends Record<string, unknown> {
+      onAction: () => void;
+      onRetry: () => void;
+    }
+
+    let actionCalled = false;
+    let retryCalled = false;
+
+    const flash = new FlashMessage<CustomFields>({
+      message: 'test',
+      sticky: true,
+      onAction: () => {
+        actionCalled = true;
+      },
+      onRetry: () => {
+        retryCalled = true;
+      },
+    });
+
+    const typedFlash = flash as FlashMessage<CustomFields> & CustomFields;
+
+    assert.strictEqual(
+      typeof typedFlash.onAction,
+      'function',
+      'onAction is preserved on the flash object',
+    );
+    assert.strictEqual(
+      typeof typedFlash.onRetry,
+      'function',
+      'onRetry is preserved on the flash object',
+    );
+
+    typedFlash.onAction();
+    typedFlash.onRetry();
+
+    assert.true(actionCalled, 'onAction callback is callable');
+    assert.true(retryCalled, 'onRetry callback is callable');
+  });
+
+  test('internal callbacks are not duplicated as custom properties', function (assert) {
+    let destroyCallCount = 0;
+
+    const flash = new FlashMessage({
+      message: 'test',
+      sticky: true,
+      onDestroy: () => {
+        destroyCallCount++;
+      },
+    });
+
+    // onDestroy should be set as the class property, not duplicated
+    assert.strictEqual(
+      typeof flash.onDestroy,
+      'function',
+      'onDestroy is set as class callback',
+    );
+
+    // Verify the callback works
+    flash.onDestroy?.();
+    assert.strictEqual(
+      destroyCallCount,
+      1,
+      'onDestroy callback works correctly',
+    );
+  });
 });

tests/unit/services/flash-messages-test.ts

@@ -567,6 +567,30 @@ module('Unit | Service | flash-messages', function (hooks) {
     assert.strictEqual(service.queue.length, 0, 'it removes the message');
   });
 
+  test('#removeBy immediately removes from queue even with extendedTimeout', function (assert) {
+    const service = this.owner.lookup(
+      'service:flash-messages',
+    ) as FlashMessagesService<{ id: string }>;
+
+    service.add({
+      message: 'system-notification',
+      id: 'test-notification',
+      sticky: true,
+      extendedTimeout: 5000,
+    });
+
+    assert.strictEqual(service.queue.length, 1, 'message is in queue');
+
+    const removed = service.removeBy('id', 'test-notification');
+
+    assert.true(removed, 'removeBy returns true');
+    assert.strictEqual(
+      service.queue.length,
+      0,
+      'message is immediately removed from queue without waiting for extendedTimeout',
+    );
+  });
+
   test('it supports custom fields via generics', function (assert) {
     const service = this.owner.lookup(
       'service:flash-messages',