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

[build/ts] transpile public code with webpack-specific ts config #21865

Merged
merged 4 commits into from
Aug 10, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion src/dev/build/tasks/clean_tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const CleanTypescriptTask = {
async run(config, log, build) {
await deleteAll(log, [
build.resolvePath('**/*.{ts,tsx,d.ts}'),
build.resolvePath('**/tsconfig.json'),
build.resolvePath('**/tsconfig*.json'),
]);
},
};
Expand Down
2 changes: 1 addition & 1 deletion src/dev/build/tasks/copy_source_task.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const CopySourceTask = {
'bin/**',
'webpackShims/**',
'config/kibana.yml',
'tsconfig.json',
'tsconfig*.json',
],
});
},
Expand Down
29 changes: 19 additions & 10 deletions src/dev/build/tasks/transpile_typescript_task.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,24 @@ export const TranspileTypescriptTask = {
description: 'Transpiling sources with typescript compiler',

async run(config, log, build) {
await exec(
log,
require.resolve('typescript/bin/tsc'),
[
'--pretty', 'true'
],
{
cwd: build.resolvePath(),
}
);
const tsConfigPaths = [
build.resolvePath('tsconfig.json'),
build.resolvePath('tsconfig.browser.json')
];

for (const tsConfigPath of tsConfigPaths) {
log.info(`Compiling`, tsConfigPath, 'project');
await exec(
log,
require.resolve('typescript/bin/tsc'),
[
'--pretty', 'true',
'--project', tsConfigPath,
],
{
cwd: build.resolvePath(),
}
);
}
},
};
12 changes: 11 additions & 1 deletion src/dev/jest/ts_transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,29 @@ import { JestConfig, TransformOptions } from 'ts-jest/dist/jest-types';

import { getTsProjectForAbsolutePath } from '../typescript';

const DEFAULT_TS_CONFIG_PATH = require.resolve('../../../tsconfig.json');
const DEFAULT_BROWSER_TS_CONFIG_PATH = require.resolve('../../../tsconfig.browser.json');

function extendJestConfigJSON(jestConfigJSON: string, filePath: string) {
const jestConfig = JSON.parse(jestConfigJSON) as JestConfig;
return JSON.stringify(extendJestConfig(jestConfig, filePath));
}

function extendJestConfig(jestConfig: JestConfig, filePath: string) {
let tsConfigFile = getTsProjectForAbsolutePath(filePath).tsConfigPath;

// swap ts config file for jest tests
if (tsConfigFile === DEFAULT_BROWSER_TS_CONFIG_PATH) {
tsConfigFile = DEFAULT_TS_CONFIG_PATH;
}

return {
...jestConfig,
globals: {
...(jestConfig.globals || {}),
'ts-jest': {
skipBabel: true,
tsConfigFile: getTsProjectForAbsolutePath(filePath).tsConfigPath,
tsConfigFile,
Copy link
Member

Choose a reason for hiding this comment

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

for whatever reason I thought we were using the browser env. any ideas if jest still includes jsdom even in node?

in any case that might explain https://github.com/elastic/kibana/pull/21811/files. I had to manually include the fetch mock browser client because it kept using node-fetch instead of window.fetch. should we not be setting anything on window there?

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, we have jest configured to include jsdom, but that only mocks out some things, mostly the document/DOM related stuff. Things like fetch() are not supported. This change is specifically necessary because our browser based config is assuming that the code will be run through webpack before being executed by a browser, which would transpile ESM into webpack requires, but when running the code in jest it is run in node right out of TypeScript, so we need to use the node config to get commonJS

Copy link
Contributor Author

@spalger spalger Aug 10, 2018

Choose a reason for hiding this comment

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

An alternative option would be to use a light webpack-like transform on these files after then are put through typescript with our browser-based config, but I don't see any reason for the additional work that would take.

},
},
};
Expand Down
4 changes: 2 additions & 2 deletions src/dev/typescript/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class Project {
private include: IMinimatch[];
private exclude: IMinimatch[];

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

if (files || !include) {
Expand All @@ -67,7 +67,7 @@ export class Project {
}

this.directory = dirname(this.tsConfigPath);
this.name = relative(REPO_ROOT, this.directory) || basename(this.directory);
this.name = name || relative(REPO_ROOT, this.directory) || basename(this.directory);
this.include = makeMatchers(this.directory, include);
this.exclude = makeMatchers(this.directory, exclude);
}
Expand Down
12 changes: 8 additions & 4 deletions src/dev/typescript/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ import { REPO_ROOT } from '../constants';
import { Project } from './project';

export const PROJECTS = [
'tsconfig.json',
'x-pack/tsconfig.json',
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
// because it takes less than 10 ms, while the other modules
// both took closer to 1000ms.
...glob.sync('packages/*/tsconfig.json', { cwd: REPO_ROOT }),
].map(path => new Project(resolve(REPO_ROOT, path)));
...glob
.sync('packages/*/tsconfig.json', { cwd: REPO_ROOT })
.map(path => new Project(resolve(REPO_ROOT, path))),
];

export function filterProjectsByFlag(projectFlag?: string) {
if (!projectFlag) {
Expand Down
3 changes: 1 addition & 2 deletions src/optimize/base_optimizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,9 @@ export default class BaseOptimizer {
transpileOnly: true,
experimentalWatchApi: true,
onlyCompileBundledFiles: true,
configFile: fromRoot('tsconfig.browser.json'),
compilerOptions: {
sourceMap: Boolean(this.sourceMaps),
target: 'es5',
module: 'esnext',
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions tsconfig.browser.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"target": "es5",
"module": "esnext",
},
"include": [
"src/**/public/**/*"
],
"exclude": [
"src/**/__fixtures__/**/*"
]
}
3 changes: 2 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"src/**/*"
],
"exclude": [
"src/**/__fixtures__/**/*"
"src/**/__fixtures__/**/*",
"src/**/public/**/*"
]
}