-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] add typescript support for the server and browser #19104
Changes from 13 commits
f26b5b4
deecb05
24f60f4
df586a0
adf5563
1bb873c
e3231ee
bd0f4e1
42efdfa
88f7a62
81044df
0f75a93
73431ed
5158ac9
8aae120
20eb968
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
const join = require('path').join; | ||
const relative = require('path').relative; | ||
const unlinkSync = require('fs').unlinkSync; | ||
const { readFileSync, writeFileSync, unlinkSync, existsSync } = require('fs'); | ||
const execFileSync = require('child_process').execFileSync; | ||
const del = require('del'); | ||
const vfs = require('vinyl-fs'); | ||
|
@@ -29,6 +29,22 @@ function removeSymlinkDependencies(root) { | |
}); | ||
} | ||
|
||
// parse a ts config file | ||
function parseTsconfig(pluginSourcePath, configPath) { | ||
const ts = require(join(pluginSourcePath, 'node_modules', 'typescript')); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: There's no need to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is Windows path support. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Node's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kind of hate using template literals for paths, it's like a phobia of mine, kind of like using concatenation to join parts of a URL... Do you care if I leave join in here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't mind |
||
|
||
const { error, config } = ts.parseConfigFileTextToJson( | ||
configPath, | ||
readFileSync(configPath, 'utf8') | ||
); | ||
|
||
if (error) { | ||
throw error; | ||
} | ||
|
||
return config; | ||
} | ||
|
||
module.exports = function createBuild( | ||
plugin, | ||
buildTarget, | ||
|
@@ -83,6 +99,45 @@ module.exports = function createBuild( | |
options | ||
); | ||
}) | ||
.then(function() { | ||
const buildConfigPath = join(buildRoot, 'tsconfig.json'); | ||
|
||
if (!existsSync(buildConfigPath)) { | ||
return; | ||
} | ||
|
||
if (!plugin.pkg.devDependencies.typescript) { | ||
throw new Error( | ||
'Found tsconfig.json file in plugin but typescript is not a devDependency.' | ||
); | ||
} | ||
|
||
// attempt to patch the extends path in the tsconfig file | ||
const buildConfig = parseTsconfig(buildSource, buildConfigPath); | ||
|
||
if (buildConfig.extends) { | ||
buildConfig.extends = join( | ||
relative(buildRoot, buildSource), | ||
buildConfig.extends | ||
); | ||
|
||
writeFileSync(buildConfigPath, JSON.stringify(buildConfig)); | ||
} | ||
|
||
execFileSync( | ||
join(buildSource, 'node_modules', '.bin', 'tsc'), | ||
['--pretty', 'true'], | ||
{ | ||
cwd: buildRoot, | ||
stdio: ['ignore', 'pipe', 'pipe'], | ||
} | ||
); | ||
|
||
del.sync([ | ||
join(buildRoot, '**', '*.{ts,tsx,d.ts}'), | ||
join(buildRoot, 'tsconfig.json'), | ||
]); | ||
}) | ||
.then(function() { | ||
const buildFiles = [relative(buildTarget, buildRoot) + '/**/*']; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,13 @@ | ||
// unless we are running a prebuilt/distributable version of | ||
// kibana, automatically transpile typescript to js before babel | ||
if (!global.__BUILT_WITH_BABEL__) { | ||
const { resolve } = require('path'); | ||
require('ts-node').register({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: With this addition, I'm not sure if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I thought that at first but then realized that once babel 7 is available it will really be babel-register again, so maybe it's fine that in the meantime it's a little more that just babel-register. |
||
transpileOnly: true, | ||
cacheDirectory: resolve(__dirname, '../../optimize/.cache/ts-node') | ||
}); | ||
} | ||
|
||
// register and polyfill need to happen in this | ||
// order and in separate files. Checkout each file | ||
// for a much more detailed explaination | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import { | |
CleanExtraBinScriptsTask, | ||
CleanExtraFilesFromModulesTask, | ||
CleanPackagesTask, | ||
CleanTypescriptTask, | ||
CleanTask, | ||
CopySourceTask, | ||
CreateArchivesSourcesTask, | ||
|
@@ -21,7 +22,8 @@ import { | |
InstallDependenciesTask, | ||
OptimizeBuildTask, | ||
RemovePackageJsonDepsTask, | ||
TranspileSourceTask, | ||
TranspileBabelTask, | ||
TranspileTypescriptTask, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I have no strong opinions here, as I think the current names get the intention across well enough even if their names aren't completely consistent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think |
||
UpdateLicenseFileTask, | ||
VerifyEnvTask, | ||
VerifyExistingNodeBuildsTask, | ||
|
@@ -76,10 +78,12 @@ export async function buildDistributables(options) { | |
await run(CopySourceTask); | ||
await run(CreateEmptyDirsAndFilesTask); | ||
await run(CreateReadmeTask); | ||
await run(TranspileSourceTask); | ||
await run(TranspileBabelTask); | ||
await run(TranspileTypescriptTask); | ||
await run(BuildPackagesTask); | ||
await run(CreatePackageJsonTask); | ||
await run(InstallDependenciesTask); | ||
await run(CleanTypescriptTask); | ||
await run(CleanPackagesTask); | ||
await run(CreateNoticeFileTask); | ||
await run(UpdateLicenseFileTask); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,4 +10,5 @@ export { | |
copyAll, | ||
getFileHash, | ||
untar, | ||
deleteAll, | ||
} from './fs'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember we removed support for jsx, was it decided to bring tsx back in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, the .jsx extension, not jsx itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll let @spalger confirm, but the same question popped into my head when I was reviewing this. The ts docs make it seem like you must use
.tsx
for jsx in typescript. I didn't check if the alternative worked because I figured we should probably do what the project recommends regardless.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's not optional in TypeScript. JSX support is only available in
.tsx
files.