feat(cli): instance proxy server #635
Conversation
mediremi
force-pushed
the
feat/proxy
branch
5 times, most recently
from
34cb28b
to
a5af15b
Compare
|
Sorry, @mediremi. I was going over and deleting some temporary comments I put in during my review and accidentally deleted the comment I was responding to (yours). That wasn't intentional. :| |
adapter/src/components/LoginModal.js
Outdated
|
|
||
| export const LoginModal = () => { | ||
| const [server, setServer] = useState( | ||
| staticUrl || window.localStorage.DHIS2_BASE_URL || '' | ||
| staticUrl || defaultUrl || window.localStorage.DHIS2_BASE_URL || '' |
There was a problem hiding this comment.
I accidentally removed a comment by @mediremi here about that the DHIS2_DEFAULT_BASE_URL can be used for Netlify PR previews.
There was a problem hiding this comment.
Would be interesting to support a dropdown list of possible backend targets (through the same proxy) here - particularly useful for netlify deploys
There was a problem hiding this comment.
We could rename it to DHIS2_DEFAULT_BASE_URLS and make it a comma-delimited string of values? If multiple values are passed we render a dropdown otherwise we render an input field?
There was a problem hiding this comment.
Are the LoginModal changes required for the proxy to work or can we defer adding REACT_APP_DHIS2_DEFAULT_BASE_URL and defaultUrl until later ?
The Netlify PR deploys do not use the start command, they are static builds anyway, so the proxy won't help there right now.
There was a problem hiding this comment.
Same with this: https://github.com/dhis2/app-platform/pull/635/files#diff-bcd4b64d0aa7ede2599b25756aa2cb9c9f0e8000857f033edb963b85eb0f54e9R47
process.env.DHIS2_DEFAULT_BASE_URL = proxyBaseUrl
Is that required for the proxy, or can it be deferred until we work on the Netlify deploy feature ?
There was a problem hiding this comment.
It's not required, it's just there to improve the dev UX by prefilling the login modal's server field to the proxy's URL.
| pathname: parsedLocation.pathname.replace( | ||
| parsedTarget.pathname, | ||
| '' | ||
| ), |
There was a problem hiding this comment.
Won't this have issues if i.e. the target pathname is /dev and there's a /dev somewhere OTHER than at the start of the string? I.e. we should probably replace /^${pathname}/ instead, so that only the first match is replaced
There was a problem hiding this comment.
When a string is passed to String.prototype.replace, only the first match is replaced (e.g. 'aaa'.replace('a', 'b') === 'baa'). For it to replace multiple matches, a regex with the g flag must be passed ('aaa'.replace(/a/g, 'b') === 'bbb').
| you specify doesn't have any domain-restricting proxy settings (such as the `SameSite=Lax`). | ||
| Make sure that the the URL of your application (`localhost:3000` in this case) | ||
| is included in the DHIS2 server's CORS whitelist (System Settings -> Access). If | ||
| your server or browser has domain-restricting settings (such as the |
There was a problem hiding this comment.
This is actually the case in modern Chrome even if the cookies haven't been explicitly set to restrict domains, since they are now SameSite by default (which is also the DHIS2 default)
# [8.0.0-beta.5](v8.0.0-beta.4...v8.0.0-beta.5) (2021-09-09) ### Features * **cli:** instance proxy server ([#635](#635)) ([9df387e](9df387e))
|
🎉 This PR is included in version 8.0.0-beta.5 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
# [8.0.0](v7.6.6...v8.0.0) (2021-09-20) ### Bug Fixes * **alerts-service:** add tests and align implementation ([dabe477](dabe477)) * **cli:** set test environment to node ([#625](#625)) ([36d311b](36d311b)) * **dependencies:** update app-runtime to v3 ([8777699](8777699)) * set jsdom as default test environment ([#624](#624)) ([2f1ba42](2f1ba42)) ### chore * **deps:** upgrade to 7.0.0 of @dhis2/ui ([b624c9e](b624c9e)) * **deps:** upgrade to styled-jsx 4.x ([8cf9e17](8cf9e17)) ### Features * **app-adapter:** align Alerts component with alerts-service and AlertBar ([bd4564c](bd4564c)) * **cli:** instance proxy server ([#635](#635)) ([9df387e](9df387e)) * bump jest to v27 ([f5015b2](f5015b2)) ### BREAKING CHANGES * **deps:** @dhis2/ui 7.x has dropped support for the deprecated entrypoints @dhis2/ui-core and @dhis2/ui-widgets. Please use @dhis2/ui to import components you need in your app. Everything from core and widgets is available. * **deps:** Upgrade to styled-jsx 4 requires that the application uses a compatible version of @dhis2/ui. * **dependencies:** This updates the app-platform to version 3 of the app-runtime. That means that this version of the app-platform will only work with apps that use version 3 of the app-runtime. * Upgrade Jest to 27.x. Please see for a list of changes: https://jestjs.io/blog/2021/05/25/jest-27
|
🎉 This PR is included in version 8.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Introduces a proxy server that can be used to test remote instances without relying on cross-site cookies.
Usage: