Repository: lingui/js-lingui
Pull request #1038 (2021-04-09 12:18:46)

fix: event emitter refactor (reverted)
This reverts commit a7aa6a6cae302c9ff595abe4f60b67e930a11b89.

vercel[bot]: This pull request is being automatically deployed with Vercel ([learn more](https://vercel.link/github-learn-more)).  
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: [https://vercel.com/lingui-js/js-lingui/GTiRq7WdiRsw8Upj8w87oc7854SW](https://vercel.com/lingui-js/js-lingui/GTiRq7WdiRsw8Upj8w87oc7854SW)  
✅ Preview: [https://js-lingui-git-fix-native-lingui-js.vercel.app](https://js-lingui-git-fix-native-lingui-js.vercel.app)


codecov[bot]: # [Codecov](https://codecov.io/gh/lingui/js-lingui/pull/1038?src=pr&el=h1) Report
> Merging [#1038](https://codecov.io/gh/lingui/js-lingui/pull/1038?src=pr&el=desc) (b7bf731) into [main](https://codecov.io/gh/lingui/js-lingui/commit/3653f6f62043a095a2babaf16a54280db9996228?el=desc) (3653f6f) will **increase** coverage by `1.84%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/lingui/js-lingui/pull/1038/graphs/tree.svg?width=650&height=150&src=pr&token=CuJ1VD5BY5)](https://codecov.io/gh/lingui/js-lingui/pull/1038?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##             main    #1038      +/-   ##
==========================================
+ Coverage   81.21%   83.06%   +1.84%     
==========================================
  Files          53       53              
  Lines        1693     1624      -69     
  Branches      455      443      -12     
==========================================
- Hits         1375     1349      -26     
+ Misses        196      158      -38     
+ Partials      122      117       -5     
```


| [Impacted Files](https://codecov.io/gh/lingui/js-lingui/pull/1038?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packages/core/src/i18n.ts](https://codecov.io/gh/lingui/js-lingui/pull/1038/diff?src=pr&el=tree#diff-cGFja2FnZXMvY29yZS9zcmMvaTE4bi50cw==) | `75.43% <ø> (ø)` | |
| [packages/core/src/eventEmitter.ts](https://codecov.io/gh/lingui/js-lingui/pull/1038/diff?src=pr&el=tree#diff-cGFja2FnZXMvY29yZS9zcmMvZXZlbnRFbWl0dGVyLnRz) | `100.00% <100.00%> (+53.75%)` | :arrow_up: |
| [packages/react/src/I18nProvider.tsx](https://codecov.io/gh/lingui/js-lingui/pull/1038/diff?src=pr&el=tree#diff-cGFja2FnZXMvcmVhY3Qvc3JjL0kxOG5Qcm92aWRlci50c3g=) | `83.87% <100.00%> (+0.53%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/lingui/js-lingui/pull/1038?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/lingui/js-lingui/pull/1038?src=pr&el=footer). Last update [3653f6f...b7bf731](https://codecov.io/gh/lingui/js-lingui/pull/1038?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

github-actions[bot]: ## size-limit report 📦 
| Path                                                               | Size                 |
| ------------------------------------------------------------------ | -------------------- |
| ./packages/core/build/cjs/core.production.min.js                   | 2.78 KB (-23.92% 🔽) |
| ./packages/detect-locale/build/cjs/detect-locale.production.min.js | 798 B (0%)           |
| ./packages/react/build/cjs/react.production.min.js                 | 4.85 KB (+0.11% 🔺)  |

Patch:
diff --git a/packages/core/src/eventEmitter.ts b/packages/core/src/eventEmitter.ts
index 4dd062cc..5abcaa5a 100644
--- a/packages/core/src/eventEmitter.ts
+++ b/packages/core/src/eventEmitter.ts
@@ -1,208 +1,31 @@
-type ListenerFunction = {
-  listener?: Function;
-} & Function
+export class EventEmitter<
+  Events extends { [name: string]: (...args: any[]) => any }
+> {
+  private readonly _events: {
+    [name in keyof Events]?: Array<Events[name]>
+  } = {}
 
-export class EventEmitter {
-  static defaultMaxListeners: number = 10;
-  maxListeners: number | undefined;
-  events: Map<string | symbol, Function[]>;
+  on(event: keyof Events, listener: Events[typeof event]): () => void {
+    if (!this._hasEvent(event)) this._events[event] = []
 
-  constructor() {
-    this.events = new Map();
+    this._events[event].push(listener)
+    return () => this.removeListener(event, listener)
   }
 
-  _addListener(
-    eventName: string | symbol,
-    listener: Function,
-    prepend: boolean
-  ): this {
-    this.emit("newListener", eventName, listener);
-    if (this.events.has(eventName)) {
-      const listeners = this.events.get(eventName) as Function[];
-      if (prepend) {
-        listeners.unshift(listener);
-      } else {
-        listeners.push(listener);
-      }
-    } else {
-      this.events.set(eventName, [listener]);
-    }
-    const max = this.getMaxListeners();
-    if (max > 0 && this.listenerCount(eventName) > max) {
-      const warning = new Error(
-        `Possible EventEmitter memory leak detected.
-         ${this.listenerCount(eventName)} ${eventName.toString()} listeners.
-         Use emitter.setMaxListeners() to increase limit`
-      );
-      warning.name = "MaxListenersExceededWarning";
-      console.warn(warning);
-    }
+  removeListener(event: keyof Events, listener: Events[typeof event]): void {
+    if (!this._hasEvent(event)) return
 
-    return this;
+    const index = this._events[event].indexOf(listener)
+    if (~index) this._events[event].splice(index, 1)
   }
 
-  addListener(eventName: string | symbol, listener: Function): this {
-    return this._addListener(eventName, listener, false);
-  }
-
-  emit(eventName: string | symbol, ...args: any[]): boolean {
-    if (this.events.has(eventName)) {
-      const listeners = (this.events.get(eventName) as Function[]).slice(); // We copy with slice() so array is not mutated during emit
-      for (const listener of listeners) {
-        try {
-          listener.apply(this, args);
-        } catch (err) {
-          this.emit("error", err);
-        }
-      }
-      return true;
-    } else if (eventName === "error") {
-      const errMsg = args.length > 0 ? args[0] : Error("Unhandled error.");
-      throw errMsg;
-    }
-    return false;
-  }
-
-  eventNames(): [string | symbol] {
-    return Array.from(this.events.keys()) as [string | symbol];
-  }
-
-  getMaxListeners(): number {
-    return this.maxListeners || EventEmitter.defaultMaxListeners;
-  }
-
-  listenerCount(eventName: string | symbol): number {
-    if (this.events.has(eventName)) {
-      return (this.events.get(eventName) as Function[]).length;
-    } else {
-      return 0;
-    }
-  }
-
-  _listeners(
-    target: EventEmitter,
-    eventName: string | symbol,
-    unwrap: boolean
-  ): Function[] {
-    if (!target.events.has(eventName)) {
-      return [];
-    }
-
-    const eventListeners: ListenerFunction[] = target.events.get(
-      eventName
-    ) as Function[];
-
-    return unwrap
-      ? this.unwrapListeners(eventListeners)
-      : eventListeners.slice(0);
-  }
-
-  unwrapListeners(arr: ListenerFunction[]): Function[] {
-    let unwrappedListeners: Function[] = new Array(arr.length) as Function[];
-    for (let i = 0; i < arr.length; i++) {
-      unwrappedListeners[i] = arr[i]["listener"] || arr[i];
-    }
-    return unwrappedListeners;
-  }
-
-  listeners(eventName: string | symbol): Function[] {
-    return this._listeners(this, eventName, true);
-  }
-
-  rawListeners(eventName: string | symbol): Function[] {
-    return this._listeners(this, eventName, false);
-  }
-
-  off(eventName: string | symbol, listener: Function): this {
-    return this.removeListener(eventName, listener);
-  }
-
-  on(eventName: string | symbol, listener: Function): this {
-    return this.addListener(eventName, listener);
-  }
-
-  once(eventName: string | symbol, listener: Function): this {
-    const wrapped: Function = this.onceWrap(eventName, listener);
-    this.on(eventName, wrapped);
-    return this;
-  }
-
-  // Wrapped function that calls EventEmitter.removeListener(eventName, self) on execution.
-  onceWrap(eventName: string | symbol, listener: Function): Function {
-    const wrapper: ListenerFunction = function (
-      this: {
-        eventName: string | symbol;
-        listener: Function;
-        rawListener: Function;
-        context: EventEmitter;
-      },
-      ...args: any[] // eslint-disable-line @typescript-eslint/no-explicit-any
-    ): void {
-      this.context.removeListener(this.eventName, this.rawListener);
-      this.listener.apply(this.context, args);
-    };
-    const wrapperContext = {
-      eventName: eventName,
-      listener: listener,
-      rawListener: wrapper,
-      context: this
-    };
-    const wrapped = wrapper.bind(wrapperContext);
-    wrapperContext.rawListener = wrapped;
-    wrapped.listener = listener;
-    return wrapped;
-  }
-
-  prependListener(eventName: string | symbol, listener: Function): this {
-    return this._addListener(eventName, listener, true);
-  }
-
-  prependOnceListener(
-    eventName: string | symbol,
-    listener: Function
-  ): this {
-    const wrapped: Function = this.onceWrap(eventName, listener);
-    this.prependListener(eventName, wrapped);
-    return this;
-  }
-
-  removeAllListeners(eventName?: string | symbol): this {
-    if (this.events === undefined) {
-      return this;
-    }
-
-    if (eventName && this.events.has(eventName)) {
-      const listeners = (this.events.get(eventName) as Function[]).slice(); // Create a copy; We use it AFTER it's deleted.
-      this.events.delete(eventName);
-      for (const listener of listeners) {
-        this.emit("removeListener", eventName, listener);
-      }
-    } else {
-      const eventList: [string | symbol] = this.eventNames();
-      eventList.map((value: string | symbol) => {
-        this.removeAllListeners(value);
-      });
-    }
-
-    return this;
-  }
+  emit(event: keyof Events, ...args: Parameters<Events[typeof event]>): void {
+    if (!this._hasEvent(event)) return
 
-  removeListener(eventName: string | symbol, listener: Function): this {
-    if (this.events.has(eventName)) {
-      const arr: Function[] = this.events.get(eventName) as Function[];
-      if (arr.indexOf(listener) !== -1) {
-        arr.splice(arr.indexOf(listener), 1);
-        this.emit("removeListener", eventName, listener);
-        if (arr.length === 0) {
-          this.events.delete(eventName);
-        }
-      }
-    }
-    return this;
+    this._events[event].map((listener) => listener.apply(this, args))
   }
 
-  setMaxListeners(n: number): this {
-    this.maxListeners = n;
-    return this;
+  private _hasEvent(event: keyof Events) {
+    return Array.isArray(this._events[event])
   }
-}
\ No newline at end of file
+}
diff --git a/packages/core/src/i18n.ts b/packages/core/src/i18n.ts
index 55e115b2..36b3aa81 100644
--- a/packages/core/src/i18n.ts
+++ b/packages/core/src/i18n.ts
@@ -53,7 +53,7 @@ type Events = {
   missing: (event: MissingMessageEvent) => void
 }
 
-export class I18n extends EventEmitter {
+export class I18n extends EventEmitter<Events> {
   _locale: Locale
   _locales: Locales
   _localeData: AllLocaleData
diff --git a/packages/react/src/I18nProvider.tsx b/packages/react/src/I18nProvider.tsx
index 4d28b379..2fda3e33 100644
--- a/packages/react/src/I18nProvider.tsx
+++ b/packages/react/src/I18nProvider.tsx
@@ -94,7 +94,7 @@ export const I18nProvider: FunctionComponent<I18nProviderProps> = ({
    * async.
    */
   React.useEffect(() => {
-    i18n.on("change", () => {
+    const unsubscribe = i18n.on("change", () => {
       setContext(makeContext())
       setRenderKey(getRenderKey())
     })
@@ -104,6 +104,7 @@ export const I18nProvider: FunctionComponent<I18nProviderProps> = ({
     if (forceRenderOnLocaleChange && renderKey === 'default') {
       console.log("I18nProvider did not render. A call to i18n.activate still needs to happen or forceRenderOnLocaleChange must be set to false.")
     }
+    return () => unsubscribe()
   }, [])
 
   if (forceRenderOnLocaleChange && renderKey === 'default') return null


Test patch:
diff --git a/packages/core/src/eventEmitter.test.ts b/packages/core/src/eventEmitter.test.ts
index 5d24f77e..0c7821ca 100644
--- a/packages/core/src/eventEmitter.test.ts
+++ b/packages/core/src/eventEmitter.test.ts
@@ -19,12 +19,12 @@ describe("@lingui/core/eventEmitter", () => {
     const listener = jest.fn()
     const emitter = new EventEmitter()
 
-    emitter.on("test", listener)
+    const unsubscribe = emitter.on("test", listener)
     emitter.emit("test", 42)
     expect(listener).toBeCalledWith(42)
 
     listener.mockReset()
-    emitter.removeAllListeners()
+    unsubscribe()
     emitter.emit("test", 42)
     expect(listener).not.toBeCalled()
   })
diff --git a/packages/react/src/I18nProvider.test.tsx b/packages/react/src/I18nProvider.test.tsx
index 3c1e38af..18258a9e 100644
--- a/packages/react/src/I18nProvider.test.tsx
+++ b/packages/react/src/I18nProvider.test.tsx
@@ -46,20 +46,20 @@ describe("I18nProvider", () => {
     expect(i18n.on).toBeCalledWith("change", expect.anything())
   })
 
-  // it("should unsubscribe for locale changes on unmount", () => {
-  //   const unsubscribe = jest.fn()
-  //   const i18n = setupI18n()
-  //   i18n.on = jest.fn(() => unsubscribe)
-
-  //   const { unmount } = render(
-  //     <I18nProvider i18n={i18n}>
-  //       <div />
-  //     </I18nProvider>
-  //   )
-  //   expect(unsubscribe).not.toBeCalled()
-  //   unmount()
-  //   expect(unsubscribe).toBeCalled()
-  // })
+  it("should unsubscribe for locale changes on unmount", () => {
+    const unsubscribe = jest.fn()
+    const i18n = setupI18n()
+    i18n.on = jest.fn(() => unsubscribe)
+
+    const { unmount } = render(
+      <I18nProvider i18n={i18n}>
+        <div />
+      </I18nProvider>
+    )
+    expect(unsubscribe).not.toBeCalled()
+    unmount()
+    expect(unsubscribe).toBeCalled()
+  })
 
   it("should re-render on locale changes", async () => {
     expect.assertions(3)