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

Dev API server #1297

Merged
merged 3 commits into from
Jun 1, 2020
Merged

Dev API server #1297

merged 3 commits into from
Jun 1, 2020

Conversation

kpuputti
Copy link
Contributor

@kpuputti kpuputti commented May 20, 2020

This PR adds a simple server that is used to start up the API routes during build time.

TODO:

  • changelog

@kpuputti kpuputti changed the base branch from master to refactor-api-router May 22, 2020 07:43
@kpuputti kpuputti marked this pull request as ready for review May 22, 2020 09:50
package.json Outdated
@@ -84,7 +85,8 @@
"audit": "yarn audit --json | node scripts/audit.js",
"clean": "rm -rf build/*",
"config": "node scripts/config.js",
"dev": "node scripts/config.js --check && sharetribe-scripts start",
"api-server": "nodemon --watch server server/apiServer.js",
"dev": "node scripts/config.js --check&&concurrently -k \"sharetribe-scripts start\" \"API_SERVER_PORT=3500 yarn run api-server\"",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it planned that you can't run the web app without api-server running on the background?

That sounds bad since it increases complexity for everybody. Could we add another script that just opens the hot-loading mode (as 'dev' did before?)

Copy link
Contributor

Choose a reason for hiding this comment

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

With the word 'complexity', I'm a bit worried about environments that we are not actively testing (e.g. running the app on windows)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, this definitely brings in complexity. The whole point of this work is to have FTW do the price calculation, and using that with flexible pricing and privileged transitions must be done in the backend.

This does take away the possibility to only deploy statically, but IMO we haven't noticed a strong need for that, and we've also always recommended Heroku as a deployment solution. I hope this paves the way to a different setup than the CRA fork, hopefully something like Next.js in the future where backend integration is a lot simpler.

We can have a separate client-only dev server, but then the process transitions would fail, and I assume that would only increase confusion and our support that is needed. That is assuming that backend-pricing becomes the new default.

server/apiServer.js Outdated Show resolved Hide resolved
Base automatically changed from refactor-api-router to master June 1, 2020 13:16
@kpuputti kpuputti merged commit f3c65c1 into master Jun 1, 2020
@kpuputti kpuputti deleted the dev-api-server branch June 1, 2020 14:15
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