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

[bootstrap] No longer build TS refs automatically #91781

Closed

Conversation

tylersmalley
Copy link
Contributor

@tylersmalley tylersmalley commented Feb 18, 2021

The building of TS references is to address Typescript being otherwise unusable in an IDE. The time this task has taken has increased greatly over the past couple of weeks as teams have migrated to project references. Quite a few folks have reached out asking how to disable this, and after discussing with @kobelb we feel that we should make it opt-in for now.

Some options which have been discussed:

  • Make it opt-out
  • Make it opt-in
  • Convert all packages to TS references and run build_ts_refs immediately as part of bootstrap instead of waiting until the end to begin.
  • No longer build Typescript references by default during bootstrap, and instead let the developer run node scripts/build_ts_refs when they feel it's necessary.
  • Move to build the references asynchronously, where the bootstrap will kick off this task. This is not completely strait-forward, as we need to figure out how to communicate this to the user and ensure we're not running multiple of these.
  • Attempt to improve caching ([ts/build-refs] implement experimental remote cache #91012 provides BUILD_TS_REFS_CACHE_ENABLE=true)
  • Build projects using Bazel (minimum 2+ months out from being able to move forward with this Migrate Kibana plugins from TS project references into Bazel #91056)
  • Address performance issues with current Typescript usage. This is hard, and not currently documented. But you can get a trace:
    • rm -rf src/core/target && ./node_modules/.bin/tsc -b src/core/tsconfig.json --extendedDiagnostics --generateTrace trace_output
    • Open chrome://tracing/ in Chrome
    • Load trace.1.json
    • The pos references a line in types.1.json

More information on project references: https://github.com/elastic/kibana/blob/master/docs/developer/best-practices/typescript.asciidoc#project-references

Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -25,7 +25,7 @@ echo "build --remote_header=x-buildbuddy-api-key=$KIBANA_BUILDBUDDY_CI_API_KEY"
### install dependencies
###
echo " -- installing node.js dependencies"
yarn kbn bootstrap --verbose
BUILD_TS_REFS_ON_BOOTSTRAP=true yarn kbn bootstrap --verbose
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brianseeders, there are issues with running scripts/build_ts_refs with symlinked node modules, which was attempted here. What would be the best way to go about running that not the main workspace and not through tasks, but not block anything downstream?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tylersmalley You could try creating a new task in tasks.groovy that does something like

// this could go directly in the tasks array in tasks.check() like this, or you could put it in a separate function
{
  dir("${env.WORKSPACE}/kibana") {
    kibanaPipeline.scriptTask('Build TS Refs', 'test/scripts/jenkins_build_ts_refs.sh')()
  }
}

This should let a task process switch back to the original workspace to run the script. Not super great, we have to be careful we don't have multiple things running out of that workspace that could conflict with each other. Other than that, it should get the job done I think.

@tylersmalley tylersmalley force-pushed the no-bootstrap-ts-refs-2 branch 3 times, most recently from c6e5159 to 4435a31 Compare February 18, 2021 01:13
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley tylersmalley added the Team:Operations Team label for Operations Team label Feb 22, 2021
@tylersmalley tylersmalley self-assigned this Feb 22, 2021
@tylersmalley
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

kibanamachine commented Feb 23, 2021

💔 Build Failed

Failed CI Steps

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @tylersmalley

@tylersmalley
Copy link
Contributor Author

Holding off on attempting to fix this for CI to see how #92513 does.

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

Successfully merging this pull request may close these issues.

4 participants