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

Use runtime publicPath for KP plugin bundles #64226

Merged
merged 6 commits into from
Apr 23, 2020

Conversation

joshdover
Copy link
Contributor

@joshdover joshdover commented Apr 22, 2020

Summary

Related to #62263

This removes the processing we do on bundles to inject the base path into bundles at runtime. Instead, bundles built by the @kbn/optimizer package will now look for a public path defined on a key in a global object: window.__kbnPublicPath__. These keys are populated by the bootstrap.js script and map each entry point id to the directory containing all of the outputs for that webpack build.

This unblocks efforts to use the brotli compression algorithm instead of gzip. This is necessary because brotli compression is slow, so we must do it at build time. This is only possible if we do not need to do any processing before serving the file to the client.

Unfortunately, this PR is mostly a "works by coincidence" situation, meaning that the integration between @kbn/optimizer and the bundle serving and bootstrapping logic must align in order for this work. There's not a great way to test this directly, but it is covered by existing e2e test coverage.

I would like to create unit tests for the ui_render_mixin and bootstrap.js script, however it seems impractical to do so until these are migrated to the Kibana Platform.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

Verified that this works as intended, super excited to get rid of the request-time public path replacements .

src/optimize/bundles_route/bundles_route.js Outdated Show resolved Hide resolved
packages/kbn-optimizer/src/worker/public_path_loader.js Outdated Show resolved Hide resolved
packages/kbn-optimizer/src/worker/webpack.config.ts Outdated Show resolved Hide resolved
@joshdover joshdover added Feature:New Platform performance release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.8.0 labels Apr 22, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@joshdover joshdover marked this pull request as ready for review April 22, 2020 21:16
@joshdover joshdover requested a review from a team as a code owner April 22, 2020 21:16
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@joshdover joshdover merged commit 3e0907f into elastic:master Apr 23, 2020
@joshdover joshdover deleted the np/public-path branch April 23, 2020 15:13
joshdover added a commit to joshdover/kibana that referenced this pull request Apr 23, 2020
spalger added a commit to mshustov/kibana that referenced this pull request Apr 28, 2020
spalger added a commit that referenced this pull request Apr 29, 2020
* convert into TS

* load plugin scripts in html body

* use buildNum as a unique Id for cache busting

* add tests for caching

* fix tests

* remove the last TODO. url should be inlined with assetss server

* this logic handled by publicPathMap on the client

* cache kbn-shared-deps as well

* attempt to fix karma tests

* always run file through replace stream

* place buildHash at begining of path, include all static files

* update bundles_route tests to inject buildNum everywhere

* fix karma config to point to right prefix

* use isDist naming throughout

* explain magic number with variables

* restore replacePublicPath option from #64226

* replace one more instance of replacePublicPath

* use promisify instead of bluebird + non-null assertions

* remove one more magic number

Co-authored-by: spalger <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
spalger added a commit to spalger/kibana that referenced this pull request Apr 29, 2020
* convert into TS

* load plugin scripts in html body

* use buildNum as a unique Id for cache busting

* add tests for caching

* fix tests

* remove the last TODO. url should be inlined with assetss server

* this logic handled by publicPathMap on the client

* cache kbn-shared-deps as well

* attempt to fix karma tests

* always run file through replace stream

* place buildHash at begining of path, include all static files

* update bundles_route tests to inject buildNum everywhere

* fix karma config to point to right prefix

* use isDist naming throughout

* explain magic number with variables

* restore replacePublicPath option from elastic#64226

* replace one more instance of replacePublicPath

* use promisify instead of bluebird + non-null assertions

* remove one more magic number

Co-authored-by: spalger <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform performance release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants