From 485be249db6786a5de80ac37e5a4b65a8a45864f Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 14 Aug 2018 11:20:16 -0700 Subject: [PATCH 1/3] [typescript] continue to use the default config in development In #21865 we tried to make a separate config file that would be used for browser TS files by excluding them from the default config file and adding a second that included them. This works fine in the build, but IDE integrations rely on being able to automatically discover a `tsconfig.json` file in a parent directory of the file being edited, which doesn't work when the tsconfig.json file is found, but excludes the file being edited. In this situation IDE integrations silently fallback to the default TSConfig settings, which don't show the types of compiler errors that will become issues in CI, like implicit any warnings. This implements the original strategy we tried in #21865 of mutating the tsconfig.json file before we run the tsc in the build so that the config files select the right files at build time. --- .../build/tasks/transpile_typescript_task.js | 30 ++++++-- src/dev/typescript/project.ts | 19 +++-- src/optimize/base_optimizer.js | 73 ++++++++++--------- tsconfig.browser.json | 5 +- tsconfig.json | 6 +- 5 files changed, 85 insertions(+), 48 deletions(-) diff --git a/src/dev/build/tasks/transpile_typescript_task.js b/src/dev/build/tasks/transpile_typescript_task.js index 43d554dc98ed4..b9f08cc21032d 100644 --- a/src/dev/build/tasks/transpile_typescript_task.js +++ b/src/dev/build/tasks/transpile_typescript_task.js @@ -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.exclude, + 'src/**/public/**/*' + ] + })); + + // compile each typescript config file + for (const tsConfigPath of [defaultProject.tsConfigPath, browserProject.tsConfigPath]) { log.info(`Compiling`, tsConfigPath, 'project'); await exec( log, diff --git a/src/dev/typescript/project.ts b/src/dev/typescript/project.ts index 7f7d12fe246f0..e0d882985bd7c 100644 --- a/src/dev/typescript/project.ts +++ b/src/dev/typescript/project.ts @@ -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) { @@ -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( diff --git a/src/optimize/base_optimizer.js b/src/optimize/base_optimizer.js index 825a1be60403c..ff8cae3fd2bc1 100644 --- a/src/optimize/base_optimizer.js +++ b/src/optimize/base_optimizer.js @@ -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. @@ -407,7 +414,7 @@ export default class BaseOptimizer { commonConfig, IS_KIBANA_DISTRIBUTABLE ? isDistributableConfig - : isSourceConfig, + : getSourceConfig(), this.uiBundles.isDevMode() ? webpackMerge(watchingConfig, supportEnzymeConfig) : productionConfig diff --git a/tsconfig.browser.json b/tsconfig.browser.json index d5cf1d99df90c..fdfc868157e0d 100644 --- a/tsconfig.browser.json +++ b/tsconfig.browser.json @@ -5,7 +5,10 @@ "module": "esnext", }, "include": [ - "src/**/public/**/*" + // in the build we populate this to include **/public/**/* but + // 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__/**/*" diff --git a/tsconfig.json b/tsconfig.json index 892b22e13dddf..5a804d5bb96be 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -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/**/*" ] } From 421e31d03ded1e389fcf2d8b5f92ee554c9589c7 Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 14 Aug 2018 12:05:31 -0700 Subject: [PATCH 2/3] [tslint] remove browser config from default projects list since it is empty --- src/dev/typescript/projects.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/dev/typescript/projects.ts b/src/dev/typescript/projects.ts index f8fc78d2b67e7..f92ebae39e517 100644 --- a/src/dev/typescript/projects.ts +++ b/src/dev/typescript/projects.ts @@ -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 From 1694452899b843cbe00fbbab8f901a0d3cc2b1fa Mon Sep 17 00:00:00 2001 From: spalger Date: Tue, 14 Aug 2018 12:11:17 -0700 Subject: [PATCH 3/3] [build/ts] fix typo --- src/dev/build/tasks/transpile_typescript_task.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dev/build/tasks/transpile_typescript_task.js b/src/dev/build/tasks/transpile_typescript_task.js index b9f08cc21032d..dd18b3d7bb01b 100644 --- a/src/dev/build/tasks/transpile_typescript_task.js +++ b/src/dev/build/tasks/transpile_typescript_task.js @@ -40,7 +40,7 @@ export const TranspileTypescriptTask = { await write(browserProject.tsConfigPath, JSON.stringify({ ...browserProject.config, include: [ - ...browserProject.config.exclude, + ...browserProject.config.include, 'src/**/public/**/*' ] }));