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

[typescript] continue to use the default config in development #21966

Merged
merged 3 commits into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions src/dev/build/tasks/transpile_typescript_task.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,36 @@
* under the License.
*/

import { exec } from '../lib';
import { exec, write } from '../lib';
import { Project } from '../../typescript';

export const TranspileTypescriptTask = {
description: 'Transpiling sources with typescript compiler',

async run(config, log, build) {
const tsConfigPaths = [
build.resolvePath('tsconfig.json'),
build.resolvePath('tsconfig.browser.json')
];
const defaultProject = new Project(build.resolvePath('tsconfig.json'));
const browserProject = new Project(build.resolvePath('tsconfig.browser.json'));

for (const tsConfigPath of tsConfigPaths) {
// update the default config to exclude **/public/**/* files
await write(defaultProject.tsConfigPath, JSON.stringify({
...defaultProject.config,
exclude: [
...defaultProject.config.exclude,
'src/**/public/**/*'
]
}));

// update the browser config file to include **/public/**/* files
await write(browserProject.tsConfigPath, JSON.stringify({
...browserProject.config,
include: [
...browserProject.config.include,
'src/**/public/**/*'
]
}));

// compile each typescript config file
for (const tsConfigPath of [defaultProject.tsConfigPath, browserProject.tsConfigPath]) {
log.info(`Compiling`, tsConfigPath, 'project');
await exec(
log,
Expand Down
19 changes: 12 additions & 7 deletions src/dev/typescript/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,7 @@ function parseTsConfig(path: string) {
throw error;
}

const files: string[] | undefined = config.files;
const include: string[] | undefined = config.include;
const exclude: string[] | undefined = config.exclude;
return { files, include, exclude };
return config;
}

function testMatchers(matchers: IMinimatch[], path: string) {
Expand All @@ -54,11 +51,19 @@ function testMatchers(matchers: IMinimatch[], path: string) {
export class Project {
public directory: string;
public name: string;
private include: IMinimatch[];
private exclude: IMinimatch[];
public config: any;

private readonly include: IMinimatch[];
private readonly exclude: IMinimatch[];

constructor(public tsConfigPath: string, name?: string) {
const { files, include, exclude = [] } = parseTsConfig(tsConfigPath);
this.config = parseTsConfig(tsConfigPath);

const { files, include, exclude = [] } = this.config as {
files?: string[];
include?: string[];
exclude?: string[];
};

if (files || !include) {
throw new Error(
Expand Down
1 change: 0 additions & 1 deletion src/dev/typescript/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import { Project } from './project';

export const PROJECTS = [
new Project(resolve(REPO_ROOT, 'tsconfig.json')),
new Project(resolve(REPO_ROOT, 'tsconfig.browser.json'), 'kibana (browser)'),
new Project(resolve(REPO_ROOT, 'x-pack/tsconfig.json')),

// NOTE: using glob.sync rather than glob-all or globby
Expand Down
73 changes: 40 additions & 33 deletions src/optimize/base_optimizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,42 +295,49 @@ export default class BaseOptimizer {
};

// when running from source transpile TypeScript automatically
const isSourceConfig = {
module: {
rules: [
{
resource: createSourceFileResourceSelector(/\.tsx?$/),
use: maybeAddCacheLoader('typescript', [
{
loader: 'ts-loader',
options: {
transpileOnly: true,
experimentalWatchApi: true,
onlyCompileBundledFiles: true,
configFile: fromRoot('tsconfig.browser.json'),
compilerOptions: {
sourceMap: Boolean(this.sourceMaps),
const getSourceConfig = () => {
// dev/typescript is deleted from the distributable, so only require it if we actually need the source config
const { Project } = require('../dev/typescript');
const browserProject = new Project(fromRoot('tsconfig.browser.json'));

return {
module: {
rules: [
{
resource: createSourceFileResourceSelector(/\.tsx?$/),
use: maybeAddCacheLoader('typescript', [
{
loader: 'ts-loader',
options: {
transpileOnly: true,
experimentalWatchApi: true,
onlyCompileBundledFiles: true,
configFile: fromRoot('tsconfig.json'),
compilerOptions: {
...browserProject.config.compilerOptions,
sourceMap: Boolean(this.sourceMaps),
}
}
}
}
]),
}
]
},
]),
}
]
},

stats: {
// when typescript doesn't do a full type check, as we have the ts-loader
// configured here, it does not have enough information to determine
// whether an imported name is a type or not, so when the name is then
// exported, typescript has no choice but to emit the export. Fortunately,
// the extraneous export should not be harmful, so we just suppress these warnings
// https://github.com/TypeStrong/ts-loader#transpileonly-boolean-defaultfalse
warningsFilter: /export .* was not found in/
},
stats: {
// when typescript doesn't do a full type check, as we have the ts-loader
// configured here, it does not have enough information to determine
// whether an imported name is a type or not, so when the name is then
// exported, typescript has no choice but to emit the export. Fortunately,
// the extraneous export should not be harmful, so we just suppress these warnings
// https://github.com/TypeStrong/ts-loader#transpileonly-boolean-defaultfalse
warningsFilter: /export .* was not found in/
},

resolve: {
extensions: ['.ts', '.tsx'],
},
resolve: {
extensions: ['.ts', '.tsx'],
},
};
};

// We need to add react-addons (and a few other bits) for enzyme to work.
Expand Down Expand Up @@ -407,7 +414,7 @@ export default class BaseOptimizer {
commonConfig,
IS_KIBANA_DISTRIBUTABLE
? isDistributableConfig
: isSourceConfig,
: getSourceConfig(),
this.uiBundles.isDevMode()
? webpackMerge(watchingConfig, supportEnzymeConfig)
: productionConfig
Expand Down
5 changes: 4 additions & 1 deletion tsconfig.browser.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
"module": "esnext",
},
"include": [
"src/**/public/**/*"
// in the build we populate this to include **/public/**/* but
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the config loader okay with comments in json? i don't think this will lint

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, typescript config can have comments, checkout tsconfig.json. It has a bunch of them and always has.

// if we did that when running from source IDEs won't properly map
// public files to this config file and instead just use the defaults, ie. "strict": false,
// **/public/**/*
],
"exclude": [
"src/**/__fixtures__/**/*"
Expand Down
6 changes: 5 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@
],
"exclude": [
"src/**/__fixtures__/**/*",
"src/**/public/**/*"
// In the build we actually exclude **/public/**/* from this config so that
// we can run the TSC on both this and the .browser version of this config
// file, but if we did it during development IDEs would not be able to find
// the tsconfig.json file for public files correctly.
// "src/**/public/**/*"
]
}