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

[Uptime] Migrate to TypeScript project references #90510

Conversation

justinkambic
Copy link
Contributor

@justinkambic justinkambic commented Feb 5, 2021

Summary

Resolves #87170.

Author checklist

  • Resolve all build errors running tsc from node_modules
  • Add project reference to references prop in tsconfig.refs.json
  • Add your plugin to references property and plugin folder to exclude property of the tsconfig.json it used to belong to (for example, for src/plugins/ it’s tsconfig.json; for x-pack/plugins/ it’s x-pack/tsconfig.json).
  • List the reference to your newly created project in all the Kibana tsconfig.json files that could import your project: tsconfig.json, test/tsconfig.json, x-pack/tsconfig.json, x-pack/test/tsconfig.json. And in all the plugin-specific tsconfig.refs.json for dependent plugins.
  • You can measure how your changes affect tsc compiler performance with node --max-old-space-size=4096 ./node_modules/.bin/tsc -p tsconfig.json --extendedDiagnostics --noEmit. Compare with master branch.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@justinkambic justinkambic added v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.12.0 labels Feb 5, 2021
@justinkambic justinkambic self-assigned this Feb 5, 2021
@justinkambic justinkambic force-pushed the uptime-87170_migrate-to-typescript-project branch from 2119e40 to 51842a6 Compare February 5, 2021 22:19
@justinkambic justinkambic marked this pull request as ready for review February 5, 2021 22:42
@justinkambic justinkambic requested a review from a team as a code owner February 5, 2021 22:42
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@justinkambic justinkambic added the release_note:skip Skip the PR/issue when compiling release notes label Feb 5, 2021
@@ -261,7 +261,25 @@
},
"state": {
"agent": null,
"checks": ,
Copy link
Contributor Author

@justinkambic justinkambic Feb 5, 2021

Choose a reason for hiding this comment

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

A little concerning that this didn't cause anything to break (until now).

export type TLSActionGroup = ActionGroup<'xpack.uptime.alerts.actionGroups.tls'>;
export type DurationAnomalyActionGroup = ActionGroup<'xpack.uptime.alerts.actionGroups.durationAnomaly'>;

export const MONITOR_STATUS: MonitorStatusActionGroup = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The compiler seemed to want these exported individually in the way we were referencing them.

@@ -12,6 +12,13 @@ import { rootReducer } from './reducers';

export type AppState = ReturnType<typeof rootReducer>;

type ComposeType = typeof compose;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was copied from another plugin's solution for providing a global type for redux devtools.

"public/**/*",
"server/**/*",
"../../../typings/**/*",
"public/**/*.json",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The glob syntax above appeared to ignore JSON files.

Copy link
Contributor

Choose a reason for hiding this comment

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

since there are just two json files, maybe it doesn't make sense to use wildcard pattern

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, we can make it more specific. The drawback will be when someone adds a dependency in the future this will need to be updated each time.

Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

Mostly good, left some minor comments / questions

"server/**/*",
"../../../typings/**/*",
"public/**/*.json",
"server/**/*.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"server/**/*.json"
"server/lib/requests/__fixtures__/monitor_charts_mock.json"

"public/**/*",
"server/**/*",
"../../../typings/**/*",
"public/**/*.json",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"public/**/*.json",
"server/lib/requests/__fixtures__/monitor_charts_mock.json"

Comment on lines 16 to 22
declare global {
interface Window {
__REDUX_DEVTOOLS_EXTENSION_COMPOSE__: ComposeType;
}
}

const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose;
Copy link
Contributor

Choose a reason for hiding this comment

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

i would prefer to use this here, this package is already part of kibana, so it would be easier to use.

import { composeWithDevTools } from 'redux-devtools-extension';
export const store = createStore(rootReducer, composeWithDevTools(applyMiddleware(sagaMW)));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👏

@shahzad31
Copy link
Contributor

@elasticmachine merge upstream

…om:justinkambic/kibana into uptime-87170_migrate-to-typescript-project
Copy link
Contributor Author

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

I just implemented review feedback and pushed 3146d9a.

"public/**/*",
"server/**/*",
"../../../typings/**/*",
"public/**/*.json",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, we can make it more specific. The drawback will be when someone adds a dependency in the future this will need to be updated each time.

Comment on lines 16 to 22
declare global {
interface Window {
__REDUX_DEVTOOLS_EXTENSION_COMPOSE__: ComposeType;
}
}

const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👏

x-pack/plugins/uptime/public/state/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

LGTM, WFG !!

Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

LGTM

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
uptime 589 590 +1

Async chunks

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

id before after diff
uptime 908.6KB 909.2KB +592.0B

Page load bundle

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

id before after diff
uptime 19.4KB 19.5KB +90.0B

History

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

@justinkambic justinkambic merged commit c7e23bf into elastic:master Feb 9, 2021
justinkambic added a commit to justinkambic/kibana that referenced this pull request Feb 9, 2021
* Add reference to Uptime plugin to root tsconfig.refs.json.

* Add Uptime path to excluded list, and reference to references prop in `x-pack/tsconfig.json`.

* Add reference to Uptime project in `x-pack/test/tsconfig.json`.

* Add `tsconfig.json` project file to Uptime.

* Fix broken JSON structure in test fixture.

* Fix broken type exports. Introduce missing types.

* Implement PR feedback.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 9, 2021
…timeline-and-rollover-info

* 'master' of github.com:elastic/kibana: (47 commits)
  [Fleet] Use TS project references (elastic#87574)
  before/beforeEach clean up (elastic#90663)
  [Vega] user should be able to set a specific tilemap service using the mapStyle property (elastic#88440)
  [Security Solution][Case] ServiceNow SIR Connector (elastic#88655)
  [Search Sessions] Enable extend from management (elastic#90558)
  [ILM] Delete phase redesign (rework) (elastic#90291)
  [APM-UI][E2E] use withGithubStatus step (elastic#90651)
  Add folding in kb-monaco and update some viewers (elastic#90152)
  [Grok Debugger] Changed test to wait for grok debugger container to exist to fix test flakiness (elastic#90543)
  Strongly typed EUI theme for styled-components (elastic#90106)
  Fix vega renovate label (elastic#90591)
  [Uptime] Migrate to TypeScript project references (elastic#90510)
  [Monitoring] Migrate data source for legacy alerts to monitoring data directly (elastic#87377)
  [Upgrade Assistant] Add A11y Tests (elastic#90265)
  [Time to Visualize] Adds functional tests for linking/unlinking panel from embeddable library (elastic#89612)
  [dev-utils/ship-ci-stats] fail when CI stats is down (elastic#90678)
  chore(NA): remove write permissions on Bazel remote cache for PRs (elastic#90652)
  chore(NA): move bazel workspace status from bash script into nodejs executable (elastic#90560)
  Use default ES distribution for functional tests (elastic#88737)
  [Alerts] Jira: Disallow labels with spaces (elastic#90548)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/timeline/timeline.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/lib/absolute_timing_to_relative_timing.test.ts
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/lib/absolute_timing_to_relative_timing.ts
@justinkambic
Copy link
Contributor Author

@justinkambic justinkambic deleted the uptime-87170_migrate-to-typescript-project branch February 9, 2021 16:15
tsullivan pushed a commit to tsullivan/kibana that referenced this pull request Feb 9, 2021
…astic#90721)

* Add reference to Uptime plugin to root tsconfig.refs.json.

* Add Uptime path to excluded list, and reference to references prop in `x-pack/tsconfig.json`.

* Add reference to Uptime project in `x-pack/test/tsconfig.json`.

* Add `tsconfig.json` project file to Uptime.

* Fix broken JSON structure in test fixture.

* Fix broken type exports. Introduce missing types.

* Implement PR feedback.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Uptime] Migrate to TypeScript project ref
5 participants