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] add typescript support for the server and browser #19104

Merged
merged 16 commits into from
May 18, 2018

Conversation

spalger
Copy link
Contributor

@spalger spalger commented May 16, 2018

Part of #18780

This PR enables TypeScript support in both the server and the client. It does not implement tslint, that will come in #19105 .

Here is a summary of how this works:

  • distributable
    • does not have any way to run TypeScript
    • all TS transpilation must happen at build time before shipping distributable
  • building
    • .ts and .tsx files are converted to .js files in place
    • Full type-check is run while TS is transpiled into .js
    • **/*.ts, **/*.tsx, **/*.d.ts and **/tsconfig.json files are stripped from the build after successful compilation
  • plugin helpers
    • plugins with a tsconfig.json file at their root will be transpiled like Kibana is and have TS files stripped while building
  • development
    • ts-node added to babel-register to auto-transpile .ts files in the server/jest/mocha
    • ts-loader transpiles .ts and .tsx files in webpack (without checking types)
    • Transpilation in both the server and optimizer is done without type checking, the process runs out of memory really fast otherwise
    • Editor integrations provide type-checking feedback to developers (vscode, atom, vim, webstorm)

To test this out grab a file and convert it to TypeScript. I find constants.js files to be easiest:

@spalger spalger added review Team:Operations Team label for Operations Team v7.0.0 v6.4.0 labels May 16, 2018
@spalger spalger added blocked and removed review labels May 16, 2018
@spalger spalger changed the base branch from implement/typescript to master May 16, 2018 02:09
@elastic elastic deleted a comment from elasticmachine May 16, 2018
@elastic elastic deleted a comment from elasticmachine May 16, 2018
@spalger spalger added review and removed blocked labels May 16, 2018
@spalger spalger force-pushed the implement/typescript branch 2 times, most recently from a60da73 to 90210f8 Compare May 16, 2018 21:12
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger
Copy link
Contributor Author

spalger commented May 17, 2018

@epixa RE no type checking in dev. This was included in a previous version of the description and I've added it back. Type checking is not supplied by the optimizer or babel-register. Adding type checking here causes things to run out of memory even on very small workloads, so instead editor integrations are leaned on for type checking information. Like you discovered, the build is the final gate and a full type-checking is done there.

@spalger
Copy link
Contributor Author

spalger commented May 17, 2018

@tylersmalley the root server directory in x-pack is not selected by the tsconfig.json file, so the typescript there is not being transpiled. Add server/**/* to

"plugins/**/*"
and it should work

@spalger
Copy link
Contributor Author

spalger commented May 17, 2018

@epixa #19105 will also be a tool people can use to run typechecking a little more efficiently than rebuilding Kibana or opening every file in their editor.

@tylersmalley
Copy link
Contributor

@spalger, that did the trick.

From the build/kibana/node_modules/x-pack/server/lib directory:

-rw-r--r--  1 tyler  staff    472 May 17 15:47 constants.js
-rw-r--r--  1 tyler  staff  10708 May 15 20:45 esjs_shield_plugin.js
-rw-r--r--  1 tyler  staff    562 May 15 20:45 get_client_shield.js
-rw-r--r--  1 tyler  staff   1642 May 15 20:45 key_case_converter.js
-rw-r--r--  1 tyler  staff    825 May 15 20:45 mirror_plugin_status.js
-rw-r--r--  1 tyler  staff   1313 May 15 20:45 parse_kibana_state.js
-rw-r--r--  1 tyler  staff    513 May 15 20:45 xpack_usage.js

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@epixa
Copy link
Contributor

epixa commented May 18, 2018

the root server directory in x-pack is not selected by the tsconfig.json file, so the typescript there is not being transpiled. Add server/**/* to

@spalger Any reason why we wouldn't just do that from the start?

@epixa
Copy link
Contributor

epixa commented May 18, 2018

RE no type checking in dev. This was included in a previous version of the description and I've added it back. Type checking is not supplied by the optimizer or babel-register. Adding type checking here causes things to run out of memory even on very small workloads, so instead editor integrations are leaned on for type checking information. Like you discovered, the build is the final gate and a full type-checking is done there.

I think I'm OK with this at least initially. It's not ideal, and maybe this is something we can fix down the line, perhaps with an upgrade of babel and/or webpack. On the fly type checking is certainly useful and expected with statically typed languages, and clearly it's possible since my editor tree is handling it. But let's not block progress for the sake of things we can do down the line.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger spalger merged commit c611206 into elastic:master May 18, 2018
@weltenwort
Copy link
Member

I gave this a try with a greenfield kibana plugin written in typescript and was able to adapt it such that the following worked:

  • run it in --dev mode (optimization time for .ts file change ~8s)
  • plugin-helpers build it such that it produces a .zip
  • kibana-plugin install that .zip and check that the plugin is loaded correctly

In doing so I stumbled across a few things:

Not tested:

  • Typescript in Mocha and Jest

Overall, great progress with a few kinks to iron out 😍

spalger pushed a commit to spalger/kibana that referenced this pull request May 18, 2018
…ic#19104)

* [typescript] add typescript support for the server and browser

* [ts-jest] upgrade to latest version

* [jest] support test.tsx files

* [jest/ts] modify `ts-jest.tsConfigFile` config based on filePath

* [types] use correct major version of minimatch types

* [jest] add ts support to x-pack jest config

* [ts/projects] fix tsconfig.json not found error message

* [optimizer/ts] use lowercase jsx option

* [tsconfig] remove ui/* alias

* [plguin-helpers] remove mention of `buildSourcePatterns`

* [plugin-helpers] expect typescript to be a devDep

* [dev/build] place transpile tasks next to each other

* [ts/x-pack] add common and server directories to ts project

* [dev/ts/project] use a limited set of globs to find tsconfig files
rhoboat pushed a commit that referenced this pull request May 18, 2018
rhoboat pushed a commit that referenced this pull request May 18, 2018
* Revert "[DOCS] Removes redundant index.asciidoc files (#19192)"

This reverts commit d11b5aa.

* Revert "[typescript] add typescript support for the server and browser (#19104)"

This reverts commit c611206.

* Revert "Option to run kibana from build for CI (#19125)"

This reverts commit 5969860.
rhoboat pushed a commit that referenced this pull request May 18, 2018
* Revert "[DOCS] Removes redundant index.asciidoc files (#19192)"

This reverts commit d11b5aa.

* Revert "[typescript] add typescript support for the server and browser (#19104)"

This reverts commit c611206.

* Revert "Option to run kibana from build for CI (#19125)"

This reverts commit 5969860.
rhoboat pushed a commit that referenced this pull request May 18, 2018
* Revert "[DOCS] Removes redundant index.asciidoc files (#19192)"

This reverts commit d11b5aa.

* Revert "[typescript] add typescript support for the server and browser (#19104)"

This reverts commit c611206.

* Revert "Option to run kibana from build for CI (#19125)"

This reverts commit 5969860.
spalger pushed a commit that referenced this pull request May 21, 2018
…19104) (#19223)

Backports the following commits to 6.x:
 - [typescript] add typescript support for the server and browser  (#19104)
@spalger
Copy link
Contributor Author

spalger commented May 21, 2018

6.x/6.4: 07f4e5a

@spalger spalger deleted the implement/typescript branch May 21, 2018 17:43
"server/**/*",
"plugins/**/*"
]
}
Copy link
Member

Choose a reason for hiding this comment

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

@spalger Ever since this file was added vscode has been complaining about errors in the workspace:

[ts] No inputs were found in config file '/Users/sqren/elastic/kibana/x-pack/tsconfig.json'. Specified 'include' paths were '["common/**/*","server/**/*","plugins/**/*"]' and 'exclude' paths were '[]'.

Any idea how to fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sqren this is probably because there aren't actually any ts files in the x-pack project yet.. Maybe you should convert something small in APM to typescript 😄 or we can remove the tsconfig.json file for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Operations Team label for Operations Team v6.4.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants