-
Notifications
You must be signed in to change notification settings - Fork 24
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
Override network, backend url through env variables after build #1510
Conversation
Deployed to https://pr-1510-aepp-base.stg.aepps.com |
src/lib/networksRegistry.js
Outdated
if (process.env.VUE_APP_NETWORK_NAME) { | ||
networks = [defaultNetwork]; | ||
} | ||
const networks = (window.overrideNetwork && [window.overrideNetwork]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you written Perl? You really love nested ternary conditions :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also because I prefer const
to let
. I did a webshop in Perl at university, but I didn't remember it, maybe it is unconscious 😃
mainNetwork, | ||
]; | ||
|
||
export const defaultNetwork = process.env.VUE_APP_NETWORK_NAME ? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do I change the networks/URLs' during development outside docker ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a network manually and switch to it. Base aepp would remember it unless the state is reset. Do you need something more permanent?
We were developing Base aepp on testnet without running a local node/mdw. The last time I tried to set up mdw for e2e tests it didn't work well.
aeternity/ae_mdw#1336
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I actually tested and developed (some very small features) locally using local node/network. After all we're looking for extension point to use the wallet in custom networks.
docker/override-network.sh
Outdated
@@ -0,0 +1,33 @@ | |||
#!/bin/sh | |||
set -e | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about using envsubst
looks much cleaner approach to me (the article says it's already in the nginx image, didn't checked) https://ledermann.dev/blog/2018/04/27/dockerize-and-configure-javascript-single-page-application/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about workflow using it. Currently, I have index.html that works with and without Docker. With envsubst
I need a template version of index.html to use in Docker, and without docker I would need something to remove the template part. Alternatively, I can maintain index.html and index.template.html with almost the same content 🤷♀️
Or do it in index.html like
<script>
window.overrideNetwork = {
name: '$VUE_APP_NETWORK_NAME',
url: '$VUE_APP_NODE_URL',
middlewareUrl: '$VUE_APP_MIDDLEWARE_URL',
explorerUrl: '$VUE_APP_EXPLORER_URL',
compilerUrl: '$VUE_APP_COMPILER_URL',
};
if (window.overrideNetwork.name.startsWith('$')) {
delete window.overrideNetwork;
}
</script>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's actually not bad idea - to drop the config template if not replaced. Outside docker, well this is why I asked above how do I do it in local dev environment. There was functionality for that which you dropped (which the process environment).
Need to handle VUE_APP_REMOTE_CONNECTION_BACKEND_URL as #1512 has troubles |
e828556
to
cb868da
Compare
cb868da
to
f34d70a
Compare
I've restored the ability to set network using env variables in vue cli, and added support to set backend url though env variables in docker. Switched to |
closes #1469
(VUE_APP_REMOTE_CONNECTION_BACKEND_URL would be handled separately)I've did some research and found several approaches.
Store configuration in a separate files (would produce an extra request)
https://www.npmjs.com/package/react-inject-env
https://www.codecentric.de/wissens-hub/blog/react-application-container-environment-aware-kubernetes-deployment
Specify env value in index.html
https://github.com/axelhzf/create-react-app-docker-environment-variables
https://iendeavor.github.io/import-meta-env/ (a complete tools set, looks overcomplicated)
Build on each container run
https://github.com/mikesparr/tutorial-react-docker
Discussions
https://stackoverflow.com/questions/52103155/reading-an-environment-variable-in-react-which-was-set-by-docker
https://stackoverflow.com/questions/49975735/rendering-an-environment-variable-to-the-browser-in-a-react-js-redux-production
facebook/create-react-app#982 (comment)
This way I can override network without rebuilding container, tested with
This PR is supported by the Æternity Crypto Foundation