From 142b69823fa422034cfc7a01f03edebf07e78830 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Fri, 17 Apr 2020 11:30:25 +0200
Subject: [PATCH] [IMP] types: do not make Env an indexed type

Before this commit, Env was an indexed type, this means that one could
write env.anything, and it would accept it as a valid type. This is
actually quite dangerous, because we lose the typing advantages for all
keys that are properly defined.

For example, if a component is defined as:

class MyComponent extends Component<Props> {
 ...
}

Then Typescript will let it use anything from the environment, even if
it is wrong.  So, most properly typed Typescript applications should use
instead a sub environment:

interface MyAppEnv extends Env {
  someKey: someValue
}

Then, the component should be defined this way:

class MyComponent extends Component<Props, MyAppEnv> {
 ...
}

Before this commit, any typos in the environment accesses would not be
noticed by typescript.
---
 src/component/component.ts        |  1 -
 src/router/route_component.ts     |  3 ++-
 src/router/router.ts              | 10 +++++++++-
 src/store.ts                      | 10 +++++++---
 tests/component/component.test.ts |  7 +++++--
 tests/store_hooks.test.ts         |  6 +++---
 6 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/component/component.ts b/src/component/component.ts
index 07161846..2d495d85 100644
--- a/src/component/component.ts
+++ b/src/component/component.ts
@@ -34,7 +34,6 @@ import { activateSheet } from "./styles";
  */
 export interface Env {
   qweb: QWeb;
-  [key: string]: any;
 }
 
 export type MountPosition = "first-child" | "last-child" | "self";
diff --git a/src/router/route_component.ts b/src/router/route_component.ts
index 4cf45e8d..71127af3 100644
--- a/src/router/route_component.ts
+++ b/src/router/route_component.ts
@@ -1,7 +1,8 @@
 import { Component } from "../component/component";
 import { xml } from "../tags";
+import { EnvWithRouter } from "./router";
 
-export class RouteComponent extends Component {
+export class RouteComponent extends Component<{}, EnvWithRouter> {
   static template = xml`
     <t>
         <t
diff --git a/src/router/router.ts b/src/router/router.ts
index b53f2128..d6c6ba87 100644
--- a/src/router/router.ts
+++ b/src/router/router.ts
@@ -49,6 +49,10 @@ interface Options {
   mode: Router["mode"];
 }
 
+export interface EnvWithRouter extends Env {
+  router: Router;
+}
+
 const paramRegexp = /\{\{(.*?)\}\}/;
 
 export class Router {
@@ -60,7 +64,11 @@ export class Router {
   routeIds: string[];
   env: RouterEnv;
 
-  constructor(env: Env, routes: Partial<Route>[], options: Options = { mode: "history" }) {
+  constructor(
+    env: Partial<EnvWithRouter>,
+    routes: Partial<Route>[],
+    options: Options = { mode: "history" }
+  ) {
     env.router = this;
     this.mode = options.mode;
     this.env = env as RouterEnv;
diff --git a/src/store.ts b/src/store.ts
index 821e0963..0099820b 100644
--- a/src/store.ts
+++ b/src/store.ts
@@ -24,6 +24,10 @@ import { onWillUpdateProps } from "./hooks";
 // Store Definition
 //------------------------------------------------------------------------------
 
+export interface EnvWithStore extends Env {
+  store: Store;
+}
+
 export type Action = ({ state, dispatch, env, getters }, ...payload: any) => any;
 export type Getter = ({ state: any, getters }, payload?) => any;
 
@@ -83,7 +87,7 @@ interface SelectorOptions {
 const isStrictEqual = (a, b) => a === b;
 
 export function useStore(selector, options: SelectorOptions = {}): any {
-  const component: Component = Component.current!;
+  const component = Component.current as Component<any, EnvWithStore>;
   const componentId = component.__owl__.id;
   const store = options.store || (component.env.store as Store);
   if (!(store instanceof Store)) {
@@ -149,11 +153,11 @@ export function useStore(selector, options: SelectorOptions = {}): any {
 }
 
 export function useDispatch(store?: Store): Store["dispatch"] {
-  store = store || (Component.current!.env.store as Store);
+  store = store || (Component.current!.env as EnvWithStore).store;
   return store.dispatch.bind(store);
 }
 
 export function useGetters(store?: Store): Store["getters"] {
-  store = store || (Component.current!.env.store as Store);
+  store = store || (Component.current!.env as EnvWithStore).store;
   return store.getters;
 }
diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts
index 969dfb57..ae8415f9 100644
--- a/tests/component/component.test.ts
+++ b/tests/component/component.test.ts
@@ -3484,11 +3484,14 @@ describe("environment and plugins", () => {
   });
 
   test("can define specific env for root components", async () => {
-    class App1 extends Component {
+    interface AppEnv extends Env {
+      test: number;
+    }
+    class App1 extends Component<{}, AppEnv> {
       static template = xml`<span></span>`;
     }
     App1.env = { test: 1 };
-    class App2 extends Component {
+    class App2 extends Component<{}, AppEnv> {
       static template = xml`<span></span>`;
     }
     App2.env = { test: 2 };
diff --git a/tests/store_hooks.test.ts b/tests/store_hooks.test.ts
index 3d457552..86edfdfe 100644
--- a/tests/store_hooks.test.ts
+++ b/tests/store_hooks.test.ts
@@ -1,5 +1,5 @@
 import { Component, Env } from "../src/component/component";
-import { Store, useStore, useDispatch, useGetters } from "../src/store";
+import { Store, useStore, useDispatch, useGetters, EnvWithStore } from "../src/store";
 import { useState } from "../src/hooks";
 import { xml } from "../src/tags";
 import { shallowEqual } from "../src/utils";
@@ -885,7 +885,7 @@ describe("connecting a component to store", () => {
       actions
     });
 
-    class TodoItem extends Component {
+    class TodoItem extends Component<{}, EnvWithStore> {
       static template = xml`
             <div class="todo">
                 <t t-esc="state.todo.title"/>
@@ -958,7 +958,7 @@ describe("connecting a component to store", () => {
       actions
     });
 
-    class TodoItem extends Component {
+    class TodoItem extends Component<{}, EnvWithStore> {
       static template = xml`
             <div class="todo">
                 <t t-esc="state.todo.title"/>