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

[kbn/optimizer] update public path before imports #67561

Merged
merged 18 commits into from
May 29, 2020

Conversation

spalger
Copy link
Contributor

@spalger spalger commented May 27, 2020

In #64226 we moved from replacing the public path in the JS source to an approach that supports setting the __webpack_public_path__ in the webpack bundles with some global variable magic. This relies on a webpack loader which injects some code into the top of each bundle entry point. This works in the kbn-ui-shared-deps where it's easier to test and all modules are imported using commonjs but didn't work for static imports in KP plugins because the public path loader was injecting code into the top of the file and webpack moved the imports up above that code, causing __webpack_public_path__ to be updated after the import statements are processed.

To fix this we no longer inject code to set __webpack_public_path__, we inject a dynamic module at the top of the entry bundle which does that, since it's an import the code stays above the rest of the imports in that file. Doing this dynamically required the use of the val-loader to dynamically generate the source for the module at compilation time.

@spalger spalger added Team:Operations Team label for Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v7.9.0 labels May 27, 2020
@spalger spalger requested a review from joshdover May 27, 2020 21:30
@spalger spalger requested a review from a team as a code owner May 27, 2020 21:30
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@spalger spalger marked this pull request as draft May 27, 2020 21:40
@spalger spalger removed the request for review from joshdover May 27, 2020 21:41
@spalger

This comment has been minimized.

@joshdover

This comment has been minimized.

@spalger

This comment has been minimized.

@spalger spalger changed the title [kbn/optimizer] apply public path loader after babel [kbn/optimizer] update public path before imports May 27, 2020
@spalger spalger marked this pull request as ready for review May 29, 2020 06:49
@spalger spalger requested a review from a team as a code owner May 29, 2020 06:49
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@spalger
Copy link
Contributor Author

spalger commented May 29, 2020

@elastic/kibana-design this includes some scss files but just for testing the optimizer.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Yep, that's fine!

Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

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

clever solution...LGTM

@spalger spalger merged commit 4020188 into elastic:master May 29, 2020
spalger pushed a commit to spalger/kibana that referenced this pull request May 29, 2020
Co-authored-by: spalger <spalger@users.noreply.github.com>
# Conflicts:
#	packages/kbn-optimizer/src/integration_tests/__snapshots__/basic_optimization.test.ts.snap
#	packages/kbn-optimizer/src/integration_tests/basic_optimization.test.ts
spalger pushed a commit to spalger/kibana that referenced this pull request May 29, 2020
Co-authored-by: spalger <spalger@users.noreply.github.com>
# Conflicts:
#	packages/kbn-optimizer/src/integration_tests/__snapshots__/basic_optimization.test.ts.snap
#	packages/kbn-optimizer/src/integration_tests/basic_optimization.test.ts
spalger added a commit that referenced this pull request Jun 1, 2020
)

Co-authored-by: spalger <spalger@users.noreply.github.com>
spalger added a commit that referenced this pull request Jun 1, 2020
)

Co-authored-by: spalger <spalger@users.noreply.github.com>
@spalger spalger deleted the fix/tweak-public-path-before-imports branch August 18, 2020 18:01
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:Operations Team label for Operations Team v7.8.0 v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants