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.

Sign up for GitHub

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

Jump to bottom

More strict plugin type definitions #205232

Merged
merged 4 commits into from
Jan 7, 2025
Merged

More strict plugin type definitions #205232

merged 4 commits into from
Jan 7, 2025

Conversation

dgieselaar
Copy link
Member

@dgieselaar dgieselaar commented Dec 29, 2024 •
edited by kibanamachine
Loading

Stricter defaults for plugin types: Plugin and CoreSetup now have empty objects as defaults instead of object which is assignable to anything basically. This catches some type errors, but my motivation for this is to allow something like:

function createPlugin ():Plugin<MySetupContract, MyStartContract, MySetupDependencies, MyStartDependencies> {
	return {
		// look ma, no additional typing necessary
		setup ( coreSetup, pluginsSetup ) {
		},
		start ( coreStart, pluginsStart ) {
		}
	}
}

@dgieselaar dgieselaar added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 v8.18.0 labels Jan 2, 2025
@dgieselaar dgieselaar self-assigned this Jan 2, 2025
@dgieselaar dgieselaar added the backport:version Backport to applied version labels label Jan 2, 2025
@dgieselaar dgieselaar marked this pull request as ready for review January 2, 2025 12:32
@dgieselaar dgieselaar requested review from a team as code owners January 2, 2025 12:32
jgowdyelastic
jgowdyelastic approved these changes Jan 2, 2025
View reviewed changes
Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

ML changes LGTM

@botelastic botelastic bot added the Team:obs-ux-management Observability Management User Experience Team label Jan 2, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

nreese
nreese approved these changes Jan 2, 2025
View reviewed changes
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

kibana-presentation changes LGTM

sphilipse
sphilipse approved these changes Jan 6, 2025
View reviewed changes
Copy link
Member

@sphilipse sphilipse left a comment

Choose a reason for hiding this comment

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

LGTM!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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
taskManager 62 61 -1

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
taskManager 6 7 +1
Unknown metric groups

API count

id before after diff
taskManager 105 104 -1

History

cc @dgieselaar

@dgieselaar dgieselaar merged commit 98ce312 into elastic:main Jan 7, 2025
9 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12654438594

@dgieselaar dgieselaar deleted the stricter-plugin-types branch January 7, 2025 15:41
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 7, 2025
@dgieselaar
More strict plugin type definitions (elastic#205232)
4df464f 
Stricter defaults for plugin types: `Plugin` and `CoreSetup` now have
empty objects as defaults instead of `object` which is assignable to
anything basically. This catches some type errors, but my motivation for
this is to allow something like:

```ts
function createPlugin ():Plugin<MySetupContract, MyStartContract, MySetupDependencies, MyStartDependencies> {
	return {
		// look ma, no additional typing necessary
		setup ( coreSetup, pluginsSetup ) {
		},
		start ( coreStart, pluginsStart ) {
		}
	}
}
```

(cherry picked from commit 98ce312)
@kibanamachine kibanamachine mentioned this pull request Jan 7, 2025
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 7, 2025
@kibanamachine @dgieselaar
[8.x] More strict plugin type definitions (#205232) (#205778)
833ef86 
# Backport

This will backport the following commits from `main` to `8.x`:
- [More strict plugin type definitions
(#205232)](#205232)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Dario
Gieselaar","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-07T15:41:15Z","message":"More
strict plugin type definitions (#205232)\n\nStricter defaults for plugin
types: `Plugin` and `CoreSetup` now have\nempty objects as defaults
instead of `object` which is assignable to\nanything basically. This
catches some type errors, but my motivation for\nthis is to allow
something like:\n\n```ts\nfunction createPlugin
():Plugin<MySetupContract, MyStartContract, MySetupDependencies,
MyStartDependencies> {\n\treturn {\n\t\t// look ma, no additional typing
necessary\n\t\tsetup ( coreSetup, pluginsSetup ) {\n\t\t},\n\t\tstart (
coreStart, pluginsStart )
{\n\t\t}\n\t}\n}\n```","sha":"98ce312ba36cbb7e4fc4a5cf16acc841c2f1b21f","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:obs-ux-management","backport:version","v8.18.0"],"title":"More
strict plugin type
definitions","number":205232,"url":"https://github.com/elastic/kibana/pull/205232","mergeCommit":{"message":"More
strict plugin type definitions (#205232)\n\nStricter defaults for plugin
types: `Plugin` and `CoreSetup` now have\nempty objects as defaults
instead of `object` which is assignable to\nanything basically. This
catches some type errors, but my motivation for\nthis is to allow
something like:\n\n```ts\nfunction createPlugin
():Plugin<MySetupContract, MyStartContract, MySetupDependencies,
MyStartDependencies> {\n\treturn {\n\t\t// look ma, no additional typing
necessary\n\t\tsetup ( coreSetup, pluginsSetup ) {\n\t\t},\n\t\tstart (
coreStart, pluginsStart )
{\n\t\t}\n\t}\n}\n```","sha":"98ce312ba36cbb7e4fc4a5cf16acc841c2f1b21f"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205232","number":205232,"mergeCommit":{"message":"More
strict plugin type definitions (#205232)\n\nStricter defaults for plugin
types: `Plugin` and `CoreSetup` now have\nempty objects as defaults
instead of `object` which is assignable to\nanything basically. This
catches some type errors, but my motivation for\nthis is to allow
something like:\n\n```ts\nfunction createPlugin
():Plugin<MySetupContract, MyStartContract, MySetupDependencies,
MyStartDependencies> {\n\treturn {\n\t\t// look ma, no additional typing
necessary\n\t\tsetup ( coreSetup, pluginsSetup ) {\n\t\t},\n\t\tstart (
coreStart, pluginsStart )
{\n\t\t}\n\t}\n}\n```","sha":"98ce312ba36cbb7e4fc4a5cf16acc841c2f1b21f"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Dario Gieselaar <[email protected]>
kowalczyk-krzysztof pushed a commit to kowalczyk-krzysztof/kibana that referenced this pull request Jan 7, 2025
@dgieselaar @kowalczyk-krzysztof
More strict plugin type definitions (elastic#205232)
6ca0be1 
Stricter defaults for plugin types: `Plugin` and `CoreSetup` now have
empty objects as defaults instead of `object` which is assignable to
anything basically. This catches some type errors, but my motivation for
this is to allow something like:

```ts
function createPlugin ():Plugin<MySetupContract, MyStartContract, MySetupDependencies, MyStartDependencies> {
	return {
		// look ma, no additional typing necessary
		setup ( coreSetup, pluginsSetup ) {
		},
		start ( coreStart, pluginsStart ) {
		}
	}
}
```
crespocarlos pushed a commit to crespocarlos/kibana that referenced this pull request Jan 8, 2025
@dgieselaar @crespocarlos
More strict plugin type definitions (elastic#205232)
78be986 
Stricter defaults for plugin types: `Plugin` and `CoreSetup` now have
empty objects as defaults instead of `object` which is assignable to
anything basically. This catches some type errors, but my motivation for
this is to allow something like:

```ts
function createPlugin ():Plugin<MySetupContract, MyStartContract, MySetupDependencies, MyStartDependencies> {
	return {
		// look ma, no additional typing necessary
		setup ( coreSetup, pluginsSetup ) {
		},
		start ( coreStart, pluginsStart ) {
		}
	}
}
```
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Jan 13, 2025
@dgieselaar @CAWilson94
More strict plugin type definitions (elastic#205232)
7d04675 
Stricter defaults for plugin types: `Plugin` and `CoreSetup` now have
empty objects as defaults instead of `object` which is assignable to
anything basically. This catches some type errors, but my motivation for
this is to allow something like:

```ts
function createPlugin ():Plugin<MySetupContract, MyStartContract, MySetupDependencies, MyStartDependencies> {
	return {
		// look ma, no additional typing necessary
		setup ( coreSetup, pluginsSetup ) {
		},
		start ( coreStart, pluginsStart ) {
		}
	}
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sphilipse sphilipse sphilipse approved these changes

@tonyghiani tonyghiani tonyghiani approved these changes

@davismcphee davismcphee davismcphee approved these changes

@rmyz rmyz rmyz approved these changes

@jgowdyelastic jgowdyelastic jgowdyelastic approved these changes

@nreese nreese nreese approved these changes

@kgeller kgeller kgeller approved these changes

@Dosant Dosant Dosant approved these changes

@shahzad31 shahzad31 shahzad31 approved these changes

@adcoelho adcoelho adcoelho approved these changes

@rudolf rudolf rudolf approved these changes

@elena-shostak elena-shostak elena-shostak approved these changes

@nickofthyme nickofthyme nickofthyme approved these changes

Assignees

@dgieselaar dgieselaar

Labels
backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.