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
Expose whitelisted config values to client-side plugin #50641
Merged
pgayvallet
merged 20 commits into
elastic:master
from
pgayvallet:kbn-41990-client-config-values
Nov 21, 2019
Merged
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
fb2b28c
introduce PluginConfigDescriptor type
pgayvallet daf09e3
inject client plugin configs in injectedMetadata
pgayvallet ac9d19b
expose client config in PluginInitializerContext
pgayvallet 99790dd
add example implementation in testbed
pgayvallet 0df8a6f
update generated doc
pgayvallet db25021
Merge remote-tracking branch 'upstream/master' into kbn-41990-client-…
pgayvallet 9b93964
only generates ui config entry for plugins exposing properties to client
pgayvallet a87b7b0
separate plugin configs from plugins
pgayvallet b6d9a83
restructure plugin services tests
pgayvallet 95f5fc4
fix test/mocks due to plugin configs api changes
pgayvallet e083125
add unit tests
pgayvallet c3c926a
update migration guide
pgayvallet 93ba773
update tsdoc
pgayvallet bd1e3bc
fix typecheck
pgayvallet 1fe445f
use sync getter for config on client side instead of observable
pgayvallet 5e34e91
change type of exposeToBrowser prop
pgayvallet ebfc428
Merge remote-tracking branch 'upstream/master' into kbn-41990-client-…
pgayvallet b147018
updates generated doc
pgayvallet cfa27e8
fix doc and address nits
pgayvallet 24575b3
Merge remote-tracking branch 'upstream/master' into kbn-41990-client-…
pgayvallet File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,12 +17,24 @@ | |
* under the License. | ||
*/ | ||
|
||
import { Plugin, CoreSetup } from 'kibana/public'; | ||
import { take } from 'rxjs/operators'; | ||
import { Plugin, CoreSetup, PluginInitializerContext } from 'kibana/public'; | ||
|
||
interface ConfigType { | ||
uiProp: string; | ||
} | ||
|
||
export class TestbedPlugin implements Plugin<TestbedPluginSetup, TestbedPluginStart> { | ||
public setup(core: CoreSetup, deps: {}) { | ||
constructor(private readonly initializerContext: PluginInitializerContext) {} | ||
|
||
public async setup(core: CoreSetup, deps: {}) { | ||
const config = await this.initializerContext.config | ||
.create<ConfigType>() | ||
.pipe(take(1)) | ||
.toPromise(); | ||
|
||
// eslint-disable-next-line no-console | ||
console.log(`Testbed plugin set up`); | ||
console.log(`Testbed plugin set up. uiProp: '${config.uiProp}'`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added an example usage in the |
||
return { | ||
foo: 'bar', | ||
}; | ||
|
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for simplicity.
optional: We can add a test that server side types can be reused.
server/config.ts
server/index.ts
public/plugin.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great and ensure the
PublicConfigType
is correctly typed. Only question I have is regarding server-to-client imports: from what I understood, we needs to be extra careful on these imports to avoid polluting the client bundles with imports dependencies, so what should be the recommended way to do this? creates atypes.ts
files at the server plugin root folder level?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds reasonable.
@kbn/config-schema
is used in node env., so we cannot recommend declaring types underpublic
orcommon
.