fix(app): support runtime base paths for subpath deployments#5969
Open
nutzlastfan wants to merge 3 commits into
Open
fix(app): support runtime base paths for subpath deployments#5969nutzlastfan wants to merge 3 commits into
nutzlastfan wants to merge 3 commits into
Conversation
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Context
This ports the runtime base-path work from a downstream OHIF 3.11 deployment into current upstream
master.The main issue is that a single build still assumes a fixed build-time
PUBLIC_URL, which breaks chunk loading, dynamic extension imports, and PWA asset resolution when the viewer is mounted under a subpath instead of/.Changes
@ohif/coreResult
A single viewer build can resolve static assets and dynamic imports correctly when served from paths like
/some-prefix/viewer/...instead of only from the site root.Notes
Greptile Summary
This PR ports runtime base-path support from a downstream OHIF 3.11 deployment, enabling a single build to be served from any subpath (e.g.
/some-prefix/viewer/) by resolving__webpack_public_path__, chunk loading, PWA assets, and dynamic extension imports at runtime rather than at build time. The approach is sound — the webpackpublicPath: ''+ runtime assignment pattern is correct, and placingsetRuntimePublicPathas the first import inindex.jsensures the public path is set before any dynamic chunk loading.Confidence Score: 4/5
Safe to merge once the open heuristic bug (viewerIndex guard, already tracked in prior threads) is resolved; remaining new findings are P2
The two new P2 findings — raw TS source import in
platform/app/src/utils/publicUrl.tsand the missing leading slash onhistoryApiFallback.index— are minor. The score is held at 4 rather than 5 because theviewerIndex >= 0correctness bug discussed in prior threads (affecting/viewer/-prefixed deployments) remains unresolved acrosspublicUrl.ts,index.html, androllbar.html.platform/core/src/utils/publicUrl.ts, platform/app/public/html-templates/index.html, platform/app/public/html-templates/rollbar.html — all share the unguarded
viewerIndex >= 0heuristic; platform/app/src/utils/publicUrl.ts — direct TS source importImportant Files Changed
viewerIndex >= 0heuristic bug (already flagged in existing threads) that resolves to the wrong path when the deployment prefix itself starts with/viewer__webpack_public_path__,window.__OHIF_BASE_PATH__, andwindow.PUBLIC_URLat boot; now correctly imports from@ohif/coreand is the first import inindex.js../../../core/src/utils/publicUrlTS source instead of@ohif/core, the same coupling that was fixed insetRuntimePublicPath.jswindow.__OHIF_BASE_PATH__and a<base>element before assets load; shares the knownviewerIndex >= 0bug with the canonical TS moduleindex.html; inherits the sameviewerIndex >= 0heuristic issuePUBLIC_URLin favour of empty-stringpublicPathso runtime__webpack_public_path__takes over;historyApiFallback.indexloses its leading/which breaks the dev-server SPA fallbackpluginImports.jswith agetRuntimeImportPathhelper that routes non-HTTP import paths throughgetPublicSubPath, correctly handling subpath deploymentsassets/…without leading/), allowing the manifest to resolve correctly under any subpathpublicUrlutilities through the package surface; straightforward additions@ohif/corepackage boundary; straightforward additionsimport './setRuntimePublicPath'as the first import, ensuring__webpack_public_path__is set before any dynamic chunk loading occursPrompt To Fix All With AI
Reviews (2): Last reviewed commit: "fix(app): derive base path from last vie..." | Re-trigger Greptile