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

Fix Typecheck & type errors #166813

Closed
wants to merge 2 commits into from
Closed

Fix Typecheck & type errors #166813

wants to merge 2 commits into from

Conversation

delanni
Copy link
Contributor

@delanni delanni commented Sep 20, 2023

Summary

These are fixes for Typescript issues that crept in in the past ~1 month, while Typescript was on a summer break.

Summary of the original error

(1) The @kbn/dev-cli-runner and @kbn/dev-proc-runner packages (used to spawn child processes during development and CI) suffered from an issue where if the child process crashes, it would be treated as if it exited with a 0 exit-code (success).

(2) We started hitting memory limits with the type-checking, resulting in out-of-memory (OOM) errors. These would terminate the type-checking code, and our runner (because of (1)) would yield a 0 exit code, thus marking the type-check step green.

With (2) being present, we basically allowed code with type-errors to pass these checks, which in turn has landed in main in the past ~4 weeks.

We've addressed (1), and we're now cleaning up the currently present type errors from the past few weeks.

@delanni delanni changed the title Fix Typecheck Fix Typecheck & type errors Sep 21, 2023
@delanni delanni mentioned this pull request Sep 22, 2023
2 tasks
Copy link
Contributor

@vadimkibana vadimkibana left a comment

Choose a reason for hiding this comment

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

Shared UX code changes LGTM.

Ikuni17 added a commit that referenced this pull request Sep 25, 2023
## Summary
This PR is the core part of #166813. The original work seems to grow
large, and we'd like to enable a preventive check beforehand to prevent
more errors from entering the codebase.

The idea is to have a selective type check that would only check changed
files' projects.
- [x] when there's no extra label, run the selective type check only on
the diffing files' projects (success:
https://buildkite.com/elastic/kibana-pull-request/builds/161837)
- [x] when the label `ci:hard-typecheck` is present, run the regular
(but now, working) full typecheck (expected to fail: )

cc: @watson

---------

Co-authored-by: Brad White <[email protected]>
Co-authored-by: Thomas Watson <[email protected]>
Co-authored-by: Thomas Watson <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
@delanni delanni added the ci:hard-typecheck Enables full typecheck on the PR label Sep 26, 2023
@jbudz
Copy link
Member

jbudz commented Sep 26, 2023

diff --git a/x-pack/plugins/osquery/cypress/tsconfig.json b/x-pack/plugins/osquery/cypress/tsconfig.json
index 7f4be5e6c8d..35da296d620 100644
--- a/x-pack/plugins/osquery/cypress/tsconfig.json
+++ b/x-pack/plugins/osquery/cypress/tsconfig.json
@@ -19,6 +19,7 @@
     "resolveJsonModule": true,
   },
   "kbn_references": [
+    { "path": "../../../test_serverless/tsconfig.json" },
     "@kbn/cypress-config",
     // cypress projects that are nested inside of other ts project use code
     // from the parent ts project in ways that can't be automatically deteceted

should fix the file overwrite errors

Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit to Ikuni17/kibana that referenced this pull request Sep 26, 2023
Ikuni17 added a commit that referenced this pull request Sep 26, 2023
## Summary

We're breaking #166813 up into
smaller PRs in the interest of getting PRs through sooner for type
fixes. These are the changes related to syncing EUI deps.
Ikuni17 added a commit that referenced this pull request Sep 26, 2023
## Summary

We're breaking #166813 up into
smaller PRs in the interest of getting PRs through sooner for type
fixes. These are the changes for Kibana Presenation.
@elastic elastic deleted a comment from kibana-ci Sep 26, 2023
@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 26, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #26 / serverless common UI Index Management Index Templates "before all" hook for "renders the index templates tab"

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
observabilityOnboarding 14 16 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
observabilityOnboarding 239.4KB 239.4KB +10.0B
securitySolution 12.8MB 12.8MB -427.0B
synthetics 860.8KB 860.8KB +25.0B
total -392.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
ml 33 32 -1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
ml 75.6KB 75.7KB +50.0B
observabilityOnboarding 5.6KB 5.5KB -57.0B
serverlessSearch 31.2KB 31.1KB -90.0B
total -97.0B
Unknown metric groups

API count

id before after diff
observabilityOnboarding 14 17 +3

ESLint disabled line counts

id before after diff
securitySolutionEss 5 6 +1

Total ESLint disabled count

id before after diff
securitySolutionEss 5 6 +1

History

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

Ikuni17 added a commit that referenced this pull request Sep 27, 2023
## Summary

We're breaking #166813 up into
smaller PRs in the interest of getting PRs through sooner for type
fixes. These are the changes for APM UI.
Ikuni17 added a commit that referenced this pull request Sep 27, 2023
## Summary

We're breaking #166813 up into
smaller PRs in the interest of getting PRs through sooner for type
fixes. These are the changes for AppEx SharedUX.
delanni added a commit that referenced this pull request Sep 27, 2023
## Summary

We're breaking #166813 up into
smaller PRs in the interest of getting PRs through sooner for type
fixes. These are the changes for Enterprise Search Frontend.

---------

Co-authored-by: Alex Szabo <[email protected]>
Ikuni17 added a commit that referenced this pull request Sep 27, 2023
## Summary

We're breaking #166813 up into
smaller PRs in the interest of getting PRs through sooner for type
fixes. These are the changes for Infra UI.
delanni pushed a commit that referenced this pull request Sep 27, 2023
## Summary

We're breaking #166813 up into
smaller PRs in the interest of getting PRs through sooner for type
fixes. These are the changes for ML UI.

---------

Co-authored-by: Dima Arnautov <[email protected]>
delanni pushed a commit that referenced this pull request Sep 27, 2023
## Summary

We're breaking #166813 up into
smaller PRs in the interest of getting PRs through sooner for type
fixes. These are the changes for Platform Deployment Management.

Co-authored-by: Kibana Machine <[email protected]>
@delanni
Copy link
Contributor Author

delanni commented Sep 27, 2023

Closing in favor of all referenced issues :)

@delanni delanni closed this Sep 27, 2023
delanni added a commit that referenced this pull request Sep 27, 2023
## Summary

We're breaking #166813 up into smaller PRs in the interest of getting
PRs through sooner for type fixes. These are the changes for
`x-pack/test_serverless`.

## Reviewers
There are no code owners for these files, so I'm using the recently
edited suggestions

---------

Co-authored-by: Alex Szabo <[email protected]>
Co-authored-by: Thomas Watson <[email protected]>
Co-authored-by: Alex Szabo <[email protected]>
Co-authored-by: Thomas Watson <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Ikuni17 added a commit that referenced this pull request Sep 27, 2023
## Summary

We're breaking #166813 up into smaller PRs in the interest of getting
PRs through sooner for type fixes. These are the changes for
`x-pack/test`.

---------

Co-authored-by: Thomas Watson <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Alex Szabo <[email protected]>
ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Sep 28, 2023
delanni added a commit that referenced this pull request Sep 28, 2023
## Summary
This is hopefully the last batch of typescript issues to be fixed,
related to #166813.

It's also re-enabling full typecheck, with this, we should be back in a
clean, typechecked main branch.

Blocked by #167428

---------

Co-authored-by: Brad White <[email protected]>
Co-authored-by: Brad White <[email protected]>
Co-authored-by: Thomas Watson <[email protected]>
Co-authored-by: Patryk Kopyciński <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
delanni added a commit to delanni/kibana that referenced this pull request Sep 29, 2023
…c#167392)

## Summary
This is hopefully the last batch of typescript issues to be fixed,
related to elastic#166813.

It's also re-enabling full typecheck, with this, we should be back in a
clean, typechecked main branch.

Blocked by elastic#167428

---------

Co-authored-by: Brad White <[email protected]>
Co-authored-by: Brad White <[email protected]>
Co-authored-by: Thomas Watson <[email protected]>
Co-authored-by: Patryk Kopyciński <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit 8a29a5e)

# Conflicts:
#	.buildkite/pipelines/pull_request/base.yml
#	.buildkite/scripts/steps/check_types_commits.sh
@delanni delanni deleted the fix-typecheck branch May 2, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:hard-typecheck Enables full typecheck on the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants