Convert services to TypeScript, part 2 (#1392)

* Convert pager service to TS

* Convert popover code to TS. Yikes

* Migrate predicate service tests to TS

* Migrate security service to TS

* Migrate sort services to TS

* Migrate timer services to TS

* Convert window event service to TS

* Export Query and ast from the defining component instead of services

* Update changelog

* Run the build in CI

* Update eui.d.ts generator to properly handle nested index files

CHANGELOG.md

@@ -1,8 +1,8 @@
 ## [`master`](https://github.com/elastic/eui/tree/master)
 
-No public interface changes since `6.0.1`.
+- Convert the other of the services to TypeScript ([#1392](https://github.com/elastic/eui/pull/1392))
 
-## [`6.0.1`](https://github.com/elastic/eui/tree/v6.0.1) 
+## [`6.0.1`](https://github.com/elastic/eui/tree/v6.0.1)
 
 **Bug fixes**
 

package.json

@@ -10,7 +10,7 @@
   "scripts": {
     "preinstall": "node ./preinstall_check",
     "start": "webpack-dev-server --port 8030 --inline --hot --config=src-docs/webpack.config.js",
-    "test-docker": "docker pull $npm_package_docker_image && docker run --rm -i -e GIT_COMMITTER_NAME=test -e GIT_COMMITTER_EMAIL=test --user=$(id -u):$(id -g) -e HOME=/tmp -v $(pwd):/app -w /app $npm_package_docker_image bash -c 'npm config set spin false && /opt/yarn*/bin/yarn && npm run test'",
+    "test-docker": "docker pull $npm_package_docker_image && docker run --rm -i -e GIT_COMMITTER_NAME=test -e GIT_COMMITTER_EMAIL=test --user=$(id -u):$(id -g) -e HOME=/tmp -v $(pwd):/app -w /app $npm_package_docker_image bash -c 'npm config set spin false && /opt/yarn*/bin/yarn && npm run test && npm run build'",
     "sync-docs": "node ./scripts/docs-sync.js",
     "build-docs": "webpack --config=src-docs/webpack.config.js",
     "build": "node ./scripts/compile-clean.js && node ./scripts/compile-eui.js && node ./scripts/compile-scss.js",
@@ -41,6 +41,7 @@
     "url": "https://github.com/elastic/eui.git"
   },
   "dependencies": {
+    "@types/lodash": "^4.14.116",
     "@types/numeral": "^0.0.25",
     "classnames": "^2.2.5",
     "core-js": "^2.5.1",
@@ -74,7 +75,6 @@
     "@types/classnames": "^2.2.6",
     "@types/enzyme": "^3.1.13",
     "@types/jest": "^23.3.9",
-    "@types/lodash": "^4.14.116",
     "@types/react": "^16.3.0",
     "@types/react-virtualized": "^9.18.6",
     "@types/uuid": "^3.4.4",
@@ -112,6 +112,7 @@
     "eslint-plugin-prettier": "^2.6.0",
     "eslint-plugin-react": "^7.4.0",
     "file-loader": "^1.1.11",
+    "findup": "^0.1.5",
     "fork-ts-checker-webpack-plugin": "^0.4.4",
     "geckodriver": "^1.11.0",
     "glob": "^7.1.2",
@@ -142,6 +143,7 @@
     "react-test-renderer": "^16.2.0",
     "redux": "^3.7.2",
     "redux-thunk": "^2.2.0",
+    "resolve": "^1.5.0",
     "rimraf": "^2.6.2",
     "sass-extract": "^2.1.0",
     "sass-extract-js": "^0.3.0",

scripts/babel/proptypes-from-ts-props/index.js

@@ -290,16 +290,20 @@ function getPropTypesForNode(node, optional, state) {
         ),
         [
           types.objectExpression(
-            node.body.map(property => {
-              const objectProperty = types.objectProperty(
-                types.identifier(property.key.name || `"${property.key.value}"`),
-                getPropTypesForNode(property.typeAnnotation, property.optional, state)
-              );
-              if (property.leadingComments != null) {
-                objectProperty.leadingComments = property.leadingComments.map(({ type, value }) => ({ type, value }));
-              }
-              return objectProperty;
-            })
+            node.body
+              // This helps filter out index signatures from interfaces,
+              // which don't translate to prop types.
+              .filter(property => property.key != null)
+              .map(property => {
+                const objectProperty = types.objectProperty(
+                  types.identifier(property.key.name || `"${property.key.value}"`),
+                  getPropTypesForNode(property.typeAnnotation, property.optional, state)
+                );
+                if (property.leadingComments != null) {
+                  objectProperty.leadingComments = property.leadingComments.map(({ type, value }) => ({ type, value }));
+                }
+                return objectProperty;
+              })
           )
         ]
       );

scripts/dtsgenerator.js

@@ -1,42 +1,65 @@
+const findup = require('findup');
+const resolve = require('resolve');
 const fs = require('fs');
 const path = require('path');
 const dtsGenerator = require('dts-generator').default;
 
 const baseDir = path.resolve(__dirname, '..');
+const srcDir = path.resolve(baseDir, 'src');
+
+function hasParentIndex(pathToFile) {
+  const isIndexFile = path.basename(pathToFile, path.extname(pathToFile)) === 'index';
+  try {
+    const fileDirectory = path.dirname(pathToFile);
+    const parentIndex = findup.sync(
+      // if this is an index file start looking in its parent directory
+      isIndexFile ? path.resolve(fileDirectory, '..') : fileDirectory,
+      'index.ts'
+    );
+    // ensure the found file is in the project
+    return parentIndex.startsWith(baseDir);
+  } catch (e) {
+    return false;
+  }
+}
 
 const generator = dtsGenerator({
   name: '@elastic/eui',
   project: baseDir,
   out: 'eui.d.ts',
   exclude: ['node_modules/**/*.d.ts', 'src/custom_typings/**/*.d.ts'],
   resolveModuleId(params) {
-    if (path.basename(params.currentModuleId) === 'index') {
+    if (path.basename(params.currentModuleId) === 'index' && !hasParentIndex(path.resolve(baseDir, params.currentModuleId))) {
       // this module is exporting from an `index(.d)?.ts` file, declare its exports straight to @elastic/eui module
       return '@elastic/eui';
     } else {
       // otherwise export as the module's path relative to the @elastic/eui namespace
-      return path.join('@elastic/eui', params.currentModuleId);
+      if (params.currentModuleId.endsWith('/index')) {
+        return path.join('@elastic/eui', path.dirname(params.currentModuleId));
+      } else {
+        return path.join('@elastic/eui', params.currentModuleId);
+      }
     }
   },
   resolveModuleImport(params) {
     // only intercept relative imports (don't modify node-modules references)
-    const isRelativeImport = params.importedModuleId[0] === '.';
+    const importFromBaseDir = path.resolve(baseDir, path.dirname(params.currentModuleId));
+    const isFromEuiSrc = importFromBaseDir.startsWith(srcDir);
+    const isRelativeImport = isFromEuiSrc && params.importedModuleId[0] === '.';
 
     if (isRelativeImport) {
-      // if importing an `index` file
-      let isModuleIndex = false;
-      if (path.basename(params.importedModuleId) === 'index') {
-        isModuleIndex = true;
-      } else {
-        const basePath = path.resolve(baseDir, path.dirname(params.currentModuleId));
-        // check if the imported module resolves to ${importedModuleId}/index.ts
-        if (!fs.existsSync(path.resolve(basePath, `${params.importedModuleId}.ts`))) {
-          // not pointing at ${importedModuleId}.ts, check if it's a directory with `index.ts`
-          if (fs.existsSync(path.resolve(basePath, `${params.importedModuleId}/index.ts`))) {
-            isModuleIndex = true;
-          }
+      // if importing from an `index` file (directly or targeting a directory with an index),
+      // then if there is no parent index file this should import from @elastic/eui
+      const importPathTarget = resolve.sync(
+        params.importedModuleId,
+        {
+          basedir: importFromBaseDir,
+          extensions: ['.ts', '.tsx'],
         }
-      }
+      );
+
+      const isIndexFile = importPathTarget.endsWith('/index.ts');
+      const isModuleIndex = isIndexFile && !hasParentIndex(importPathTarget);
 
       if (isModuleIndex) {
         // importing an `index` file, in `resolveModuleId` above we change those modules to '@elastic/eui'
@@ -55,11 +78,14 @@ const generator = dtsGenerator({
   },
 });
 
-// strip any `/// <reference` lines from the generated eui.d.ts
+// 1. strip any `/// <reference` lines from the generated eui.d.ts
+// 2. replace any import("src/...") declarations to import("@elastic/src/...")
 generator.then(() => {
   const defsFilePath = path.resolve(baseDir, 'eui.d.ts');
   fs.writeFileSync(
     defsFilePath,
-    fs.readFileSync(defsFilePath).toString().replace(/\/\/\/\W+<reference.*/g, '')
+    fs.readFileSync(defsFilePath).toString()
+      .replace(/\/\/\/\W+<reference.*/g, '') // 1.
+      .replace(/import\(\"src\/(.*?)\"\)/g, 'import("@elastic/eui/src/$1")') // 2.
   );
 });

src/components/index.js

@@ -256,7 +256,9 @@ export {
 } from './progress';
 
 export {
-  EuiSearchBar
+  EuiSearchBar,
+  Query,
+  Ast
 } from './search_bar';
 
 export {

src/components/search_bar/index.js

@@ -1,4 +1,3 @@
-export { EuiSearchBar } from './search_bar';
-export { QueryType } from './search_bar';
+export { EuiSearchBar, QueryType, Query, Ast } from './search_bar';
 export { SearchBoxConfigPropTypes } from './search_box';
 export { SearchFiltersFiltersType } from './search_filters';

src/components/search_bar/search_bar.js

@@ -13,6 +13,8 @@ import PropTypes from 'prop-types';
 import { Query } from './query';
 import { EuiFlexItem } from '../flex/flex_item';
 
+export { Query, AST as Ast } from './query';
+
 export const QueryType = PropTypes.oneOfType([ PropTypes.instanceOf(Query), PropTypes.string ]);
 
 export const SearchBarPropTypes = {

src/index.d.ts

@@ -1,4 +1,3 @@
-/// <reference path="./services/index.d.ts" />
 /// <reference path="./components/index.d.ts" />
 /// <reference path="./themes/index.d.ts" />
 /// <reference path="./test/index.d.ts" />

src/services/index.js → src/services/index.ts

@@ -43,12 +43,6 @@ export {
   Pager
 } from './paging';
 
-// TODO: Migrate these services into the services directory.
-export {
-  Query,
-  AST as Ast,
-} from '../components/search_bar/query';
-
 export {
   Random
 } from './random';

src/services/paging/pager.test.js → src/services/paging/pager.test.ts

@@ -5,14 +5,17 @@ import {
 describe('Pager', () => {
   describe('constructor', () => {
     test('throws error if missing totalItems', () => {
+      // @ts-ignore
       expect(() => new Pager()).toThrow();
     });
 
     test('throws error if missing itemsPerPage', () => {
+      // @ts-ignore
       expect(() => new Pager(10)).toThrow();
     });
 
     test('throws error if non-number initialPageIndex', () => {
+      // @ts-ignore
       expect(() => new Pager(10, 3, 'invalid argument')).toThrow();
     });
   });
@@ -21,7 +24,8 @@ describe('Pager', () => {
     const totalItems = 10;
     const itemsPerPage = 3;
     const initialPageIndex = 1;
-    let pager;
+    // Initialising this to a Pager straight away keep TS happy.
+    let pager = new Pager(totalItems, itemsPerPage, initialPageIndex);
 
     beforeEach(() => {
       pager = new Pager(totalItems, itemsPerPage, initialPageIndex);
@@ -152,7 +156,7 @@ describe('Pager', () => {
         expect(pager.getLastItemIndex()).toBe(3);
       });
 
-      test(`doesn't update current page`, () => {
+      test("doesn't update current page", () => {
         pager.setItemsPerPage(2);
         expect(pager.getCurrentPageIndex()).toBe(initialPageIndex);
       });
@@ -161,6 +165,7 @@ describe('Pager', () => {
 
   describe('behavior', () => {
     describe('when there are no items', () => {
+      // TODO
     });
   });
 });

src/services/paging/pager.js → src/services/paging/pager.ts

@@ -1,33 +1,45 @@
+import { isNumber } from '../predicate';
+
 export class Pager {
-  constructor(totalItems, itemsPerPage, initialPageIndex = 0) {
-    if (isNaN(parseInt(totalItems, 10))) {
+  currentPageIndex: number;
+  firstItemIndex: number;
+  itemsPerPage: number;
+  lastItemIndex: number;
+  totalItems: number;
+  totalPages: number;
+
+  constructor(totalItems: number, itemsPerPage: number, initialPageIndex: number = 0) {
+    if (!isNumber(totalItems) || isNaN(totalItems)) {
       throw new Error('Please provide a number of totalItems');
     }
 
-    if (isNaN(parseInt(itemsPerPage, 10))) {
+    if (!isNumber(itemsPerPage) || isNaN(itemsPerPage)) {
       throw new Error('Please provide a number of itemsPerPage');
     }
 
-    if (isNaN(parseInt(initialPageIndex, 10))) {
+    if (!isNumber(initialPageIndex) || isNaN(initialPageIndex)) {
       throw new Error('Please provide a number of initialPageIndex');
     }
 
-    this.totalItems = totalItems;
-    this.itemsPerPage = itemsPerPage;
     this.currentPageIndex = initialPageIndex;
+    this.firstItemIndex = -1;
+    this.itemsPerPage = itemsPerPage;
+    this.lastItemIndex = -1;
+    this.totalItems = totalItems;
+    this.totalPages = 0;
 
     this.update();
   }
 
-  setTotalItems = (totalItems) => {
+  setTotalItems = (totalItems: number) => {
     this.totalItems = totalItems;
     this.update();
-  };
+  }
 
-  setItemsPerPage = (itemsPerPage) => {
+  setItemsPerPage = (itemsPerPage: number) => {
     this.itemsPerPage = itemsPerPage;
     this.update();
-  };
+  }
 
   isPageable = () => this.firstItemIndex !== -1;
 
@@ -45,13 +57,13 @@ export class Pager {
 
   goToNextPage = () => {
     this.goToPageIndex(this.currentPageIndex + 1);
-  };
+  }
 
   goToPreviousPage = () => {
     this.goToPageIndex(this.currentPageIndex - 1);
-  };
+  }
 
-  goToPageIndex = (pageIndex) => {
+  goToPageIndex = (pageIndex: number) => {
     this.currentPageIndex = pageIndex;
     this.update();
   }
@@ -73,5 +85,5 @@ export class Pager {
     // Find the range of visible items on the current page.
     this.firstItemIndex = this.currentPageIndex * this.itemsPerPage;
     this.lastItemIndex = Math.min(this.firstItemIndex + this.itemsPerPage, this.totalItems) - 1;
-  };
+  }
 }