Transpile packages on demand, validate all TS projects #146212
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spalger
force-pushed
the
implement/babel-register-packages
branch
16 times, most recently
from
274b3f4 to
cd4ea7a
Compare
spalger
force-pushed
the
implement/babel-register-packages
branch
2 times, most recently
from
b05725d to
723f0e1
Compare
mistic
reviewed
Dec 6, 2022
packages/core/test-helpers/core-test-helpers-kbn-server/BUILD.bazel
Outdated
Show resolved
Hide resolved
spalger
force-pushed
the
implement/babel-register-packages
branch
from
5a2270c to
82c4612
Compare
|
I've made some progress on this PR. CI is now green and it is in sync with the latest main. The only thing that catch my eye with this PR and that probably need a bit more work is related to CI workers for Jest unit tests. They doubled with this PR and the CI is consistent around 1h30mins |
spalger
force-pushed
the
implement/babel-register-packages
branch
from
2fad901 to
92014fb
Compare
… their own worker
…r/kibana into implement/babel-register-packages
opauloh
approved these changes
Dec 22, 2022
💛 Build succeeded, but was flaky
Failed CI Steps
Test FailuresMetrics [docs]Module Count
Public APIs missing comments
Any counts in public APIs
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
ESLint disabled in files
ESLint disabled line counts
References to deprecated APIs
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
kibanamachine
added
v8.7.0
backport:skip
2 tasks
1 task
cnasikas
added a commit
that referenced
this pull request
Dec 23, 2022
## Summary In the new edit tags & edit assignees flyouts, I use `emotion` to add CSS that makes the flyout body 100% in height. PR #146212 introduces some changes where `emotion` runs in development mode. Before this PR it wasn't. This had as a result to break the styles in Cases. Our e2e tests did not catch it because all tests in CI run in production mode. ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
afgomez
pushed a commit
to afgomez/kibana
that referenced
this pull request
Dec 27, 2022
Since elastic#146212 we use a development build of React. This has surfaced some existing problems in the `UptimePageTemplateComponent`, namely a `useMemo` hook that was calling other hooks inside it by caching a component. The wrapped component seems to have some extra styling for mobile users. For now we will remove this change to make the Uptime plugin work again and tackle the issues with mobile rendering in a different PR.
5 tasks
|
It looks like this change broke Kibana debugging workflow When I run and pausing application on break-point, in source file where break-point placed all imports are replaced with here is original codebase imports /*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import type {
AlertInstanceContext,
AlertInstanceState,
RuleExecutorServices,
} from '@kbn/alerting-plugin/server';
import { firstValueFrom } from 'rxjs';
import type { LicensingPluginSetup } from '@kbn/licensing-plugin/server';
import { getFilter } from '../get_filter';
import type { BucketHistory } from '../alert_suppression/group_and_bulk_create';
import { groupAndBulkCreate } from '../alert_suppression/group_and_bulk_create';
import { searchAfterAndBulkCreate } from '../search_after_bulk_create';
import type { ITelemetryEventsSender } from '../../../telemetry/sender';
import type { UnifiedQueryRuleParams } from '../../rule_schema';
import type { ExperimentalFeatures } from '../../../../../common/experimental_features';
import { buildReasonMessageForQueryAlert } from '../reason_formatters';
import { withSecuritySpan } from '../../../../utils/with_security_span';
import { scheduleNotificationResponseActions } from '../../rule_response_actions/schedule_notification_response_actions';
import type { SetupPlugins } from '../../../../plugin_contract';
import type { RunOpts } from '../../rule_types/types';and these are imports during break-point:
It makes very complicated to navigate between source files, as names of imported methods are changed and no longer referencing a file where they were imported from. Is it a known issue or any workarounds available at this moment? Thanks |
|
Sorry about that @vitaliidm, fixed in #148407 |
spalger
pushed a commit
that referenced
this pull request
Jan 4, 2023
…48407) In #146212 the default `sourceMaps` config for babel register changed from `both` to `true`, which creates the source maps but does not embed them in the compiled output so that dev-tools are able to inspect them even though they are not on the filesystem. cc @vitaliidm
spalger
pushed a commit
that referenced
this pull request
Feb 7, 2023
After #146212 it feels like the babel-register cache is getting invalidated more frequently for some reason. The current version of the cache only stores a single cache entry for each file path, which shouldn't be too big of a problem but with these changes several versions of a file will be cached. The performance seems about equal, but because the cache contains multiple versions of a single file we should spend less time transpiling files when switching branches often. --------- Co-authored-by: kibanamachine <[email protected]>
|
We are experiencing issues with external plugin build and webpack, see forum post . Could these changes be causing our plugin failure? We do not see this issue when running kibana dev only with a build image. We have no issues with Kibana 8.6.2.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dearest Reviewers 👋
I've been working on this branch with @mistic and @tylersmalley and we're really confident in these changes. Additionally, this changes code in nearly every package in the repo so we don't plan to wait for reviews to get in before merging this. If you'd like to have a concern addressed, please feel free to leave a review, but assuming that nobody raises a blocker in the next 24 hours we plan to merge this EOD pacific tomorrow, 12/22.
We'll be paying close attention to any issues this causes after merging and work on getting those fixed ASAP. 🚀
The operations team is not confident that we'll have the time to achieve what we originally set out to accomplish by moving to Bazel with the time and resources we have available. We have also bought ourselves some headroom with improvements to babel-register, optimizer caching, and typescript project structure.
In order to make sure we deliver packages as quickly as possible (many teams really want them), with a usable and familiar developer experience, this PR removes Bazel for building packages in favor of using the same JIT transpilation we use for plugins.
Additionally, packages now use
kbn_references(again, just copying the dx from plugins to packages).Because of the complex relationships between packages/plugins and in order to prepare ourselves for automatic dependency detection tools we plan to use in the future, this PR also introduces a "TS Project Linter" which will validate that every tsconfig.json file meets a few requirements:
the chain of base config files extended by each config includes
tsconfig.base.jsonand nottsconfig.jsonthe
includeconfig is used, and notfilesthe
excludeconfig includestarget/**/*the
outDircompiler option is specified astarget/typesnone of these compiler options are specified:
declaration,declarationMap,emitDeclarationOnly,skipLibCheck,target,pathsall references to other packages/plugins use their pkg id, ie:
only packages/plugins which are imported somewhere in the ts code are listed in
kbn_referencesThis linter is not only validating all of the tsconfig.json files, but it also will fix these config files to deal with just about any violation that can be produced. Just run
node scripts/ts_project_linter --fixlocally to apply these fixes, or let CI take care of automatically fixing things and pushing the changes to your PR.No bazel? Does that mean no packages??
Nope! We're still doing packages but we're pretty sure now that we won't be using Bazel to accomplish the 'distributed caching' and 'change-based tasks' portions of the packages project.
This PR actually makes packages much easier to work with and will be followed up with the bundling benefits described by the original packages RFC. Then we'll work on documentation and advocacy for using packages for any and all new code.
We're pretty confident that implementing distributed caching and change-based tasks will be necessary in the future, but because of recent improvements in the repo we think we can live without them for at least a year.
Wait, there are still BUILD.bazel files in the repo
Yes, there are still three webpack bundles which are built by Bazel: the
@kbn/ui-shared-deps-npmDLL,@kbn/ui-shared-deps-srcexternals, and the@kbn/monacoworkers. These three webpack bundles are still created during bootstrap and remotely cached using bazel. The next phase of this project is to figure out how to get the package bundling features described in the RFC with the current optimizer, and we expect these bundles to go away then. Until then any package that is used in those three bundles still needs to have a BUILD.bazel file so that they can be referenced by the remaining webpack builds.