Skip to content

Commit

Permalink
Merge pull request jaegertracing#348 from jaegertracing/plexus-type-defs
Browse files Browse the repository at this point in the history
Export plexus type declarations, remove Neutrino
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
  • Loading branch information
tiffon authored Mar 19, 2019
2 parents 6688c5e + c53f5cd commit cdf1e22
Show file tree
Hide file tree
Showing 24 changed files with 766 additions and 403 deletions.
5 changes: 3 additions & 2 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
build/**/*.js
coverage/**/*.js
build
coverage
packages/plexus/src/LayoutManager/layout.worker*js*
14 changes: 7 additions & 7 deletions BUILD.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ ESLint is being used to lint the repo, as a whole. Within `./packages/plexus` (f

#### `typescript`

The TypeScript package (e.g. `typescript`) is **not for compiling** TypeScript source but is included for the purpose of bolstering the linting of TypeScript files. `tsc` catches quite a few issues that ESLint does not pick up on.
In the project root, `typescript` is used to bolster the linting of TypeScript files. `tsc` catches quite a few issues that ESLint does not pick up on.

In `./packages/plexus`, `typescript` is used to generate type declarations for the ES module build. See [`./packages/plexus/BUILD.md`](packages/plexus/BUILD.md#typescript---emitdeclarationonly) for details.

### Workspaces

Expand Down Expand Up @@ -66,7 +68,7 @@ Runs the `lint` and `test` scripts.

## `.eslintrc.js`

Pretty basic. Needs to be cleaned up. The `airbnb` configuration needs to be updated.
Pretty basic.

Note: This configuration is extended by `./packages/plexus/.eslintrc.js`.

Expand Down Expand Up @@ -96,16 +98,14 @@ A few notable [compiler settings](http://www.typescriptlang.org/docs/handbook/co
* [dom.iterable](https://github.com/Microsoft/TypeScript/blob/master/lib/lib.dom.iterable.d.ts)
* [webworker](https://github.com/Microsoft/TypeScript/blob/master/lib/lib.webworker.d.ts)
* `skipLibCheck` - Maybe worth reevaluating in the future
* `strict` - Very important
* `strict` - Important
* `noEmit` - We're using this for linting, after all
* `include` - We've included `./typgings` here because it turned out to be a lot simpler than configuring `types`, `typeRoots` and `paths`

## `typings/{custom.d.ts, index.d.ts}`

This is relevant for `./packages/plexus/src/LayoutManager/layout.worker.js` and the `viz.js` package.
This is relevant for `./packages/plexus/src/LayoutManager/getLayout.tsx` and the `viz.js` package.

I wasn't able to get much in the line of error messaging, so I'm pretty vague on this.

The version of `viz.js` in use (1.8.1) ships with an `index.d.ts` file, but it has some issues. I was able to define alternate type declarations for `viz.js` in `./typings/custom.d.ts` and referring `./typings/index.d.ts` to `./typings/custom.d.ts`. I also changed the import for `viz.js` to `viz.js/viz.js`, which is importing it's main file, directly, instead of implicitly.

For `./packages/plexus/src/LayoutManager/layout.worker.js`, webpack (in `./packages/plexus`) is set up to use the [`worker-loader`](https://github.com/webpack-contrib/worker-loader) to load the file. This converts it to a `WebWorker` by instantiating the `WebWorker` with the source as a `Blob`. Consequently, `layout.worker.js` is implemented in the context of a `WorkerGlobalScope` but consumed as if it's a regular class that extends `Worker`. To deal with this mismatch, a **webpack alias** was created mapping `worker-alias/` to `./packages/plexus/src`. This allowed a type declaration to be defined for `worker-alias/*` as a subclass of `Worker`.
The version of `viz.js` in use (1.8.1) ships with an `index.d.ts` file, but it has some issues. `./typings/custom.d.ts` defines an alternate type declaration for `viz.js` by targeting `viz.js/viz.js`. It was necessary use `./typings/index.d.ts` to refer to `./typings/custom.d.ts`. Then, importing the modules main file, which is `viz.js/viz.js`, will use the alternate type declaration.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@
"prepare": "lerna run --stream --sort prepublishOnly",
"prettier-comment": "https://github.com/yarnpkg/yarn/issues/6300",
"prettier":
"./node_modules/prettier/bin-prettier.js --write '{.,scripts}/*.{js,json,md,ts,tsx}' 'packages/*/{src,demo/src}/**/!(react-vis).{css,js,json,md,ts,tsx}' 'packages/*/*.{css,js,json,md,ts,tsx}'",
"./node_modules/prettier/bin-prettier.js --write '{.,scripts}/*.{js,json,md,ts,tsx}' 'packages/*/{src,demo/src}/**/!(layout.worker.bundled|react-vis).{css,js,json,md,ts,tsx}' 'packages/*/*.{css,js,json,md,ts,tsx}'",
"prettier-lint":
"./node_modules/prettier/bin-prettier.js --list-different '{.,scripts}/*.{js,json,md,ts,tsx}' 'packages/*/{src,demo/src}/**/!(react-vis).{css,js,json,md,ts,tsx}' 'packages/*/*.{css,js,json,md,ts,tsx}'",
"./node_modules/prettier/bin-prettier.js --list-different '{.,scripts}/*.{js,json,md,ts,tsx}' 'packages/*/{src,demo/src}/**/!(layout.worker.bundled|react-vis).{css,js,json,md,ts,tsx}' 'packages/*/*.{css,js,json,md,ts,tsx}'",
"test": "lerna run test",
"tsc-lint": "tsc",
"tsc-lint-debug": "tsc --listFiles",
Expand Down
2 changes: 1 addition & 1 deletion packages/jaeger-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"source-map-explorer": "^1.6.0"
},
"dependencies": {
"@jaegertracing/plexus": "0.0.1-dev.4",
"@jaegertracing/plexus": "0.1.0",
"antd": "3.8.0",
"chance": "^1.0.10",
"classnames": "^2.2.5",
Expand Down
4 changes: 4 additions & 0 deletions packages/plexus/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,13 @@ module.exports = {
'prettier/@typescript-eslint',
],
rules: {
// use @typescript-eslint/no-useless-constructor to avoid null error on *.d.ts files
'no-useless-constructor': 0,

'@typescript-eslint/explicit-function-return-type': 0,
'@typescript-eslint/explicit-member-accessibility': 0,
'@typescript-eslint/no-explicit-any': 0,
'@typescript-eslint/no-useless-constructor': 1,
'@typescript-eslint/prefer-interface': 0,
},
};
4 changes: 3 additions & 1 deletion packages/plexus/.gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
build
dist
lib
src/LayoutManager/layout.worker*js*
100 changes: 0 additions & 100 deletions packages/plexus/.neutrinorc.js

This file was deleted.

99 changes: 76 additions & 23 deletions packages/plexus/BUILD.md
Original file line number Diff line number Diff line change
@@ -1,44 +1,97 @@
# Build Considerations: `./packages/plexus`

The package is implemented in TypeScript and must be compiled to JavaScript. The build output goes into `./build` as one main file, `./build/index.js`.
**Note:** File references are relative to `./packages/plexus`; `./` refers to `./packages/plexus`.

**Note:** File references are relative to `./packages/plexus`. E.g. `./` is actually referring to `./packages/plexus`, relative to the project root.
The package is implemented in TypeScript and must be compiled to JavaScript.

## Neutrino
There are three build scenarios and one pre-step common to all of them.

[Neutrino](https://master.neutrinojs.org/) is used to build `plexus`. The [release candidate for `v9`](https://github.com/neutrinojs/neutrino/milestone/6) is used.
Pre-step:

While Neutrino intends to allow for "zero initial configuration", that turned out not to be the case for `plexus`.
* Bundle `./src/LayoutManager/layout.worker.tsx` to a UMD module which can initialize a `WebWorker` from a `Blob` URL

`.neutrinorc.js` has a fair bit of configuration. It's well commented, so it won't be rehashed, here.
Build scenarios:

### `webpack.config.js`
* Production ES modules
* **This is the project's default export as `./lib/index.js`.** This build is not bundled and therefore does not use Webpack.
* Production UMD module
* Webpack dev server
* Runs `./demo/src/index.tsx` which has a few example graphs.

**Configures the webpack entry point.** Neutrino is used to generate the webpack config, but there were challenges in using it to set a non-Neutrino default entry point.
The pre-step, which they all require, is to bundle `./src/LayoutManager/layout.worker.tsx` via the `worker-loader` Webpack loader.

### `worker-alias`
## Babel

`worker-alias` is set up as a webpack alias in `.neutrinorc.js`. This alias matches a declaration in the project root: `../../typings/custom.d.ts`. See `../../BUILD.md` for details.
Babel is used to transpile the TypeScript for all scenarios and the pre-step. See `babel.config.js` for specifics.

## `package.json`
The production ES module build is not bundled and therefore does not use Webpack.

### Script `prepublishOnly`
## Webpack

Executed after `yarn install` is run in the project root.
Webpack is used to:

### Dependencies (dev and otherwise)
* Bundle `./src/LayoutManager/layout.worker.tsx` so we can have a `WebWorker` without forcing folks to deal with an additional JavaScript asset
* Bundle the production UMD module
* Run the Webpack dev server during development

#### `viz.js@1.8.1`
`./webpack-factory.js` is used to generate the Webpack configurations for each scenario.

This specific version of [viz.js](https://github.com/mdaines/viz.js) is used to avoid a regression. Meanwhile, [looks like `2.x.x`](https://github.com/mdaines/viz.js/issues/120#issuecomment-389281407) has recovered a lot of ground; [GitHub ticket](https://github.com/jaegertracing/jaeger-ui/issues/339) to upgrade.
## TypeScript `--emitDeclarationOnly`

Compiling TypeScript via Babel does not allow for type declarations to be generated. So, `tsc` is used with `./tsconfig.json` to generate the type defs.

This only applies to the ES module production build, output to `./lib`.

Note: `./tsconfig.json` does not extend `../../tsconfig.json`.

## Pre-step: `layout.worker`

#### `worker-loader`
`./src/LayoutManager/layout.worker.tsx` is intended to be loaded as a `WebWorker`. To be able to load it as a `Worker` without requiring an extra JS file, Webpack and the [`worker-loader`](https://github.com/webpack-contrib/worker-loader) loader are used to bundle it into a UMD module, `./src/LayoutManager/layout.worker.bundled.js`.

[`worker-loader`](https://github.com/webpack-contrib/worker-loader) is a webpack plugin that allows a file to be imported as a class which, when instantiated, creates a WebWorker from the underlying source.
Within the UMD module, `layout.worker.tsx` (and everything bundled into it) is turned into a `Blob` URL that's used to initialize a `WebWorker`.

#### `@babel/preset-typescript`
The resultant UMD module can be initialized as a class:

TypeScript is compiled through Babel via [`@babel/preset-typescript`](https://babeljs.io/docs/en/babel-preset-typescript). **The TypeScript compiler is not used to compile TS files.** Note: `tsc` is used to compliment the use of ESLint to lint TypeScript, though.
```ts
import LayoutWorker from './layout.worker.bundled';

const leWorker = new LayoutWorker();

leWorker.postMessage(...);
```

> It's all fun and games until type checking loses an eye.
To make sure we don't end up with an implicit `any`, `layout.worker.bundled.d.ts` provides a type declaration:

```ts
class LayoutWorker extends Worker { ... }
```

## `package.json`

### Scripts

* `build` — Generates the UMD bundle and ES module production builds
* `prepublishOnly` — Executed after `yarn install` is run in the project root; runs the `build` script
* `start` — Starts the Webpack dev server and watches all files, including `layout.worker`

The `_tasks/*` scripts are not intended to be run, directly.

* `_tasks/clean/*`
* Remove generated files
* `_tasks/bundle-worker`
* Generates the `layout.worker` UMD bundle
* `_tasks/build/*`
* Generates the production ES and UMD builds
* `_tasks/dev-server`
* Starts the Webpack dev server

### Dependencies (dev and otherwise)

#### `viz.js@1.8.1`

This specific version of [viz.js](https://github.com/mdaines/viz.js) is used to avoid a regression. Meanwhile, [looks like `2.x.x`](https://github.com/mdaines/viz.js/issues/120#issuecomment-389281407) has recovered a lot of ground; [GitHub ticket](https://github.com/jaegertracing/jaeger-ui/issues/339) to upgrade.

#### `jest@23.6.0`

Expand All @@ -48,8 +101,8 @@ Jest is not actually be used, yet. Present as a placeholder. ([Ticket](https://g

Configures ESLint for TypeScript. ESLint is executed from the project root, but this file is merged with the project root `.eslintrc.js` and overrides where there is overlap.

Uses [`@typescript-eslint/parser`](https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/parser) as the parser.
`prettier/@typescript-eslint` needs to be last in the `extends` so it overrides the formatting rules from `plugin:@typescript-eslint/recommended`.

It's worth noting that the `tsconfig.json` is in the project root and `tsconfigRootDir: '.'` refers to the project root.
Uses [`@typescript-eslint/parser`](https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/parser) as the parser.

`prettier/@typescript-eslint` needs to be last in the `extends` so it overrides the formatting rules from `plugin:@typescript-eslint/recommended`.
The `tsconfigRootDir: '.'` refers to the project root because that is where ESLint is executed, from. And, the `tsconfig.json` referred to by `./.eslintrc.js` is that in the project root.
53 changes: 53 additions & 0 deletions packages/plexus/babel.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright (c) 2019 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

function getBabelConfig(api) {
const env = api.env();
return {
plugins: [
'@babel/plugin-syntax-dynamic-import',
[
'babel-plugin-transform-react-remove-prop-types',
{
removeImport: true,
},
],
[
'@babel/plugin-proposal-class-properties',
{
loose: true,
},
],
],
presets: [
[
'@babel/preset-env',
{
// this should match the settings in jaeger-ui/package.json
targets: ['>0.5%', 'not dead', 'not ie <= 11', 'not op_mini all'],
},
],
[
'@babel/preset-react',
{
development: env === 'development',
useBuiltIns: true,
},
],
'@babel/preset-typescript',
],
};
}

module.exports = getBabelConfig;
13 changes: 13 additions & 0 deletions packages/plexus/demo/template.ejs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!DOCTYPE html>
<html lang="<%= htmlWebpackPlugin.options.lang %>">

<head>
<meta charset="utf-8">
<title><%= htmlWebpackPlugin.options.title %></title>
</head>

<body>
<div id="<%= htmlWebpackPlugin.options.appMountId %>"></div>
</body>

</html>
Loading

0 comments on commit cdf1e22

Please sign in to comment.