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

Add a method for plugins to add injected vars to every app #9071

Merged
merged 8 commits into from
Nov 18, 2016

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Nov 15, 2016

Several times while developing x-pack we have needed to little bits of data from the server at UI Bootstrap time. This can currently be solved by either putting a static set of variables in your injectDefaultVars() ui-export, or make a request at bootstrap and delay everything for the request to complete.

The later has become increasingly necessary, and has lead to several race-conditions as we try to expose a synchronous API for data that is loaded asynchronously (unless it's cached).

This change adds a replaceInjectedVars(originalInjectedVars, request, server) uiExport type that receives the injectedVars created by the app that is being rendered, the current request, and the hapi server object. The function should return new injectedVars (or a promise for one) to be sent to the view in this render.

@w33ble and @lukasolson worked with me on the design, so we could make sure that it made sense from a plugin authors perspective

Copy link
Contributor

@w33ble w33ble left a comment

Choose a reason for hiding this comment

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

I think jsdom is a little heavy for the use here, but otherwise LGTM

import KbnServer from '../../server/kbn_server';

const getInjectedVarsFromResponse = (resp) => {
const document = jsdom.jsdom(resp.payload);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you just use jquery to do the css selection and read the data from the DOM here? And if not, wouldn't something like cheerio be a lighter solution for it?

Copy link
Contributor Author

@spalger spalger Nov 17, 2016

Choose a reason for hiding this comment

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

These tests are run in node, so jQuery isn't an option. cheerio looks perfect

@lukasolson
Copy link
Member

LGTM!

@spalger spalger merged commit dd46f75 into elastic:master Nov 18, 2016
elastic-jasper added a commit that referenced this pull request Nov 18, 2016
Backports PR #9071

**Commit 1:**
[uiExports] add replaceInjectedVars() export type

* Original sha: 7ad237c
* Authored by spalger <[email protected]> on 2016-11-14T22:11:47Z

**Commit 2:**
[ui] do not assume es plugin is always there

* Original sha: 5ac383d
* Authored by spalger <[email protected]> on 2016-11-15T00:28:45Z

**Commit 3:**
[server/status] fix typo

* Original sha: 3a97f69
* Authored by spalger <[email protected]> on 2016-11-15T00:29:19Z

**Commit 4:**
[ui] add errror handling to /app/{id} endpoint

* Original sha: 33aa9de
* Authored by spalger <[email protected]> on 2016-11-15T00:29:43Z

**Commit 5:**
[ui] add tests for replaceInjectedVars()

* Original sha: 4471453
* Authored by spalger <[email protected]> on 2016-11-15T00:30:14Z

**Commit 6:**
Merge branch 'master' of github.com:elastic/kibana into implement/extend-injected-vars

* Original sha: d316ff5
* Authored by spalger <[email protected]> on 2016-11-17T01:54:58Z

**Commit 7:**
[npm] swap out jsdom with cheerio

* Original sha: c0e6a62
* Authored by spalger <[email protected]> on 2016-11-17T01:58:01Z

**Commit 8:**
[server/ui] continue extender => replacer rename

* Original sha: 3d833e8
* Authored by spalger <[email protected]> on 2016-11-17T20:55:11Z
spalger pushed a commit that referenced this pull request Nov 18, 2016
Backports PR #9071

**Commit 1:**
[uiExports] add replaceInjectedVars() export type

* Original sha: 7ad237c
* Authored by spalger <[email protected]> on 2016-11-14T22:11:47Z

**Commit 2:**
[ui] do not assume es plugin is always there

* Original sha: 5ac383d
* Authored by spalger <[email protected]> on 2016-11-15T00:28:45Z

**Commit 3:**
[server/status] fix typo

* Original sha: 3a97f69
* Authored by spalger <[email protected]> on 2016-11-15T00:29:19Z

**Commit 4:**
[ui] add errror handling to /app/{id} endpoint

* Original sha: 33aa9de
* Authored by spalger <[email protected]> on 2016-11-15T00:29:43Z

**Commit 5:**
[ui] add tests for replaceInjectedVars()

* Original sha: 4471453
* Authored by spalger <[email protected]> on 2016-11-15T00:30:14Z

**Commit 6:**
Merge branch 'master' of github.com:elastic/kibana into implement/extend-injected-vars

* Original sha: d316ff5
* Authored by spalger <[email protected]> on 2016-11-17T01:54:58Z

**Commit 7:**
[npm] swap out jsdom with cheerio

* Original sha: c0e6a62
* Authored by spalger <[email protected]> on 2016-11-17T01:58:01Z

**Commit 8:**
[server/ui] continue extender => replacer rename

* Original sha: 3d833e8
* Authored by spalger <[email protected]> on 2016-11-17T20:55:11Z
@epixa epixa added v5.1.1 and removed v5.1.0 labels Dec 8, 2016
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
)

Backports PR elastic#9071

**Commit 1:**
[uiExports] add replaceInjectedVars() export type

* Original sha: 7ad237c
* Authored by spalger <[email protected]> on 2016-11-14T22:11:47Z

**Commit 2:**
[ui] do not assume es plugin is always there

* Original sha: 5ac383d
* Authored by spalger <[email protected]> on 2016-11-15T00:28:45Z

**Commit 3:**
[server/status] fix typo

* Original sha: 3a97f69
* Authored by spalger <[email protected]> on 2016-11-15T00:29:19Z

**Commit 4:**
[ui] add errror handling to /app/{id} endpoint

* Original sha: 33aa9de
* Authored by spalger <[email protected]> on 2016-11-15T00:29:43Z

**Commit 5:**
[ui] add tests for replaceInjectedVars()

* Original sha: 4471453
* Authored by spalger <[email protected]> on 2016-11-15T00:30:14Z

**Commit 6:**
Merge branch 'master' of github.com:elastic/kibana into implement/extend-injected-vars

* Original sha: d316ff5
* Authored by spalger <[email protected]> on 2016-11-17T01:54:58Z

**Commit 7:**
[npm] swap out jsdom with cheerio

* Original sha: c0e6a62
* Authored by spalger <[email protected]> on 2016-11-17T01:58:01Z

**Commit 8:**
[server/ui] continue extender => replacer rename

* Original sha: 3d833e8
* Authored by spalger <[email protected]> on 2016-11-17T20:55:11Z

Former-commit-id: 2b04dde
@spalger spalger deleted the implement/extend-injected-vars branch August 18, 2020 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants