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 <base> and config webpack at runtime to allow path prefix #198

Merged
merged 2 commits into from Mar 21, 2018

Conversation

tiffon
Copy link
Member

@tiffon tiffon commented Mar 19, 2018

Fix #42.

Allows a path prefix to be used through four changes:

  • package.json#homepage is set to ".". When create-react-app generates the build index.html the static assets are linked via relative paths.
  • Added a <base> element to the index.html template. All relative paths (e.g. the static assets referenced in HTML) are relative to the value of <base>, which is set to "/" in index.html, but the query-service can replace it (see Provide a means to define a path-prefix for the UI jaeger#745). It's worth noting, this only affects relative paths.
  • The publicPath value in webpack is configured at runtime via the __webpack_public_path__ global. The value is derived from the <base>, and ensures any chunks or other assets (like the logo) are loaded relative to the site prefix.
  • The pathPrefix used for fetch() calls is based on the <base> value instead of package.json#homepage

As a side note, this PR can be merged before any work is done in the query-service.

Signed-off-by: Joe Farro <joef@uber.com>
@ghost ghost assigned tiffon Mar 19, 2018
@ghost ghost added the review label Mar 19, 2018
@codecov
Copy link

codecov bot commented Mar 19, 2018

Codecov Report

Merging #198 into master will increase coverage by <.01%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #198      +/-   ##
==========================================
+ Coverage   91.46%   91.47%   +<.01%     
==========================================
  Files          95       96       +1     
  Lines        2133     2135       +2     
  Branches      435      436       +1     
==========================================
+ Hits         1951     1953       +2     
  Misses        161      161              
  Partials       21       21
Impacted Files Coverage Δ
src/index.js 0% <ø> (ø) ⬆️
src/api/jaeger.js 97.22% <ø> (-0.08%) ⬇️
src/utils/prefix-url.js 100% <100%> (ø) ⬆️
src/site-prefix.js 80% <80%> (ø)
...neViewer/TimelineHeaderRow/TimelineViewingLayer.js 92.59% <0%> (+1.85%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6181a29...8d62be8. Read the comment docs.

@yurishkuro
Copy link
Member

what's the issue with code coverage? the numbers are always confusing does 88.88% of diff hit refer to coverage of the new/changed code?

Signed-off-by: Joe Farro <joef@uber.com>
@tiffon
Copy link
Member Author

tiffon commented Mar 19, 2018

what's the issue with code coverage? the numbers are always confusing does 88.88% of diff hit refer to coverage of the new/changed code?

@yurishkuro I think that means of the modified lines 88.88% of them were hit by code coverage.

@tiffon tiffon merged commit e20963b into master Mar 21, 2018
@ghost ghost removed the review label Mar 21, 2018
@yurishkuro yurishkuro deleted the issue-42-base-element-site-prefix branch January 29, 2020 15:07
vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this pull request Jul 5, 2021
…tracing#198)

* Fix #42 use <base> and config webpack at runtime

Signed-off-by: Joe Farro <joef@uber.com>

* Discourage manually editing the base URL

Signed-off-by: Joe Farro <joef@uber.com>

Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No support for Jaeger behind a reverse proxy
3 participants