12 factor refactor to fix #25

[chrisvfritz]

@alexdilley Does this resolve the issues you noticed? What are your thoughts?

[alexdilley]

The VUE_APP_API_BASE_URL env var is only really an optimisation for production builds. It generates a bundle with the value of the variable statically embedded. So something like the following:

create({baseURL:"https://api.example.com"})

as opposed to a more verbose lookup against the environment:

create({baseURL:Object({
  NODE_ENV:"production",
  API_BASE_URL:"https://api.example.com",
  ...
}).API_BASE_URL})

I wasn't anticipating a refactor to use this over the equivalent without the VUE_APP_ prefix, except possibly a mention in the docs that this is favourable in certain circumstances for production builds (albeit a reiteration of what's in the vue-cli docs).

I think my issue relates to the way the axios node module as a whole was being mocked for Jest tests when presumably for most projects a module in app code will configure axios with things such as a base URL for each environment (based on environment variables). I couldn't see an entry point in the project currently where this can be configured without such an extension (e.g. how would builds for production have the prod API URL embedded). In my case, I've created src/utils/http.js which exports my axios instance. Perhaps this is needed alongside a src/utils/__mocks__/http.js counterpart. However, I'm finding that the testURL setting in jest.config.js is actually the thing that's directing requests and that tests/units/__mocks__/axios.js is redundant???

I hope I'm making some sense 😕 I'll spend some time looking more into this.

The move from calling jest directly in scripts to calling via vue-cli-service does look like right since it'll utilise users' .env files (that's the 12-factor bit imo).

[alexdilley]

Just looked at the difference between builds using .env files for per-environment configuration. I'm mistaken: it is the case that only vars prefixed with VUE_APP_ are available in client-side code. I think I misread the built code previously: it's not the above, but the following:

create({baseURL:Object({
  NODE_ENV:"production",
  BASE_URL:"/"
}).API_BASE_URL})

d'oh! (NODE_ENV and BASE_URL being the special variables exposed by vue-cli.)

So what you're doing in this PR probably does make sense. But let me come back to you after some further investigation.

I'm still curious as to what your recommendation might be for axios setup in production...is it the "module in src/utils" idea?

[chrisvfritz]

I'm still curious as to what your recommendation might be for axios setup in production...is it the "module in src/utils" idea?

I just added a couple comments to the issue which may answer that question. The tldr: when the API is always on the same domain (including subdomain) as the frontend, then no special hacks are necessary in production environments.

Let me know if that makes sense. 🙂

And actually, now I'm second-guessing my rename of API_BASE_URL to VUE_APP_API_BASE_URL. I did that so that it could be available on the frontend, but I think actually accessing it in the frontend source would actually be an anti-pattern. Do you see something I might be missing or should I change it back?

[alexdilley]

As discussed on #25, I don't think the refactor to use a VUE_APP_ prefix is necessary unless the context is production (where a configured baseURL would need to be embedded into client-side code). The rest LGTM 👍

[chrisvfritz]

Thanks! You were quite right that the axios mock was made unnecessary by providing a testUrl, so I've removed that as well. 🙂 Even simpler!