Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert services to TypeScript, part 2 #1392

Merged
merged 11 commits into from
Jan 3, 2019
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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**

Expand Down
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
24 changes: 14 additions & 10 deletions scripts/babel/proptypes-from-ts-props/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
})
)
]
);
Expand Down
62 changes: 44 additions & 18 deletions scripts/dtsgenerator.js
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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.
);
});
4 changes: 3 additions & 1 deletion src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,9 @@ export {
} from './progress';

export {
EuiSearchBar
EuiSearchBar,
Query,
Ast
} from './search_bar';

export {
Expand Down
3 changes: 1 addition & 2 deletions src/components/search_bar/index.js
Original file line number Diff line number Diff line change
@@ -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';
2 changes: 2 additions & 0 deletions src/components/search_bar/search_bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
1 change: 0 additions & 1 deletion src/index.d.ts
Original file line number Diff line number Diff line change
@@ -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" />
16 changes: 0 additions & 16 deletions src/services/index.d.ts

This file was deleted.

6 changes: 0 additions & 6 deletions src/services/index.js → src/services/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Expand All @@ -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);
Expand Down Expand Up @@ -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);
});
Expand All @@ -161,6 +165,7 @@ describe('Pager', () => {

describe('behavior', () => {
describe('when there are no items', () => {
// TODO
});
});
});
40 changes: 26 additions & 14 deletions src/services/paging/pager.js → src/services/paging/pager.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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();
}
Expand All @@ -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;
};
}
}
Loading