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

Replace the Ember config by a React config #9

Merged
merged 4 commits into from
Jul 14, 2017
Merged

Replace the Ember config by a React config #9

merged 4 commits into from
Jul 14, 2017

Conversation

eugenk
Copy link
Member

@eugenk eugenk commented Jun 30, 2017

This allows us to deploy the react frontend. The ontohub-frontend repository is cloned to the local machine, the correct branch is checked out there and then it's built locally. We don't have any dependencies on the server for the frontend.

Unfortunately, we need to override some of the git tasks for this, but it's for a good cause.

@eugenk
Copy link
Member Author

eugenk commented Jul 2, 2017

Do we want to deploy the .map files as well? If they are only for debugging purposes, I would rather omit them in the live version (ontohub.org) and only upload them to the development servers (staging.ontohub.org and develop.ontohub.org).

@phyrog
Copy link
Contributor

phyrog commented Jul 2, 2017

I don't think there is any harm in uploading it for the live version too. They are only loaded when having the developer tools open, so no impact for normal users, but significantly improves debuggability if we need to debug the live version. Besides, the code is open source, so it's not like the sourcemaps reveal anything new.

Only impact is the harddrive space needed, which we should have plenty of.

@phyrog
Copy link
Contributor

phyrog commented Jul 7, 2017

I think we should make some changes to the frontend to make it easier to inject config options during build (especially the backend host address). We're currently using the file src/config.js to set config values. Unless we're planning to add lots of build-time config options, it would probably be easier to use the .env files.

@eugenk
Copy link
Member Author

eugenk commented Jul 9, 2017

Good point. My response in your PR applies here, too. This means that I need to add some code to this PR that changes the REACT_APP_BACKEND_HOST in the .env file to the address that the config/mixins/applications/*.rb file configures. Will do this after your PR is merged.

@phyrog
Copy link
Contributor

phyrog commented Jul 10, 2017

Isn't it possible to just set an environment variable during building? Then you wouldn't need to change the .env file.

@eugenk
Copy link
Member Author

eugenk commented Jul 10, 2017

Done with 56e3e57. I deployed the current frontend to https://staging.ontohub.org, but it indefinitely sends GetVersion and CurrentUser requests.

@eugenk
Copy link
Member Author

eugenk commented Jul 10, 2017

This happens because the backend responded with an error:

{"errors":[{"message":"Field 'me' doesn't exist on type 'Query'","locations":[{"line":2,"column":3}],"fields":["query CurrentUser","me"]}]}

Updating the backend on staging fixed this.

@phyrog
Copy link
Contributor

phyrog commented Jul 10, 2017

Can you open an issue in the frontend repo for this? Infinite loops should not happen...

@eugenk
Copy link
Member Author

eugenk commented Jul 13, 2017

Done with ontohub/ontohub-frontend#94

Copy link
Contributor

@phyrog phyrog left a comment

Choose a reason for hiding this comment

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

@eugenk eugenk merged commit aaf919f into master Jul 14, 2017
@eugenk eugenk deleted the react_config branch July 14, 2017 11:29
@eugenk eugenk removed the in review label Jul 14, 2017
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.

2 participants