Make webpack-dev-server work with authentication when using a local API instance

RESOLVED FIXED

Status

P2
normal
RESOLVED FIXED
2 years ago
7 months ago

People

(Reporter: emorley, Assigned: emorley)

Tracking

(Blocks: 1 bug)

Details

Attachments

(1 attachment)

When the Neutrino work landed it unfortunately regressed the full stack development workflow. Instead of being able to run `./manage.py runserver` and start working immediately, one either has to:
* build the UI in full manually (like occurs in production)
* use `yarn start:local` which builds the UI but then watches/rebuilds as necessary

The latter approach is still much slower than using webpack-dev-server as normal, leaves the files in `dist/` which clutters up grep results, and requires hacky workarounds in the Neutrino configuration files.

Instead I've figured out a way that we can replace `start:local` with the standard webpack-dev-server that proxies requests and still works with authentication (which was the blocking for doing so before, since cross-domain didn't work).
I have this working, but am trying to work around bugs with webpack-dev-server and the proxy path globbing (to try and make requests to eg `/static/` and `/admin/` work too, and not just `/api/`), since I appear to be hitting the same issue as:
https://github.com/webpack/webpack-dev-server/issues/458#issuecomment-251871148

Even though the docs say this should be supported:
https://webpack.js.org/configuration/dev-server/#devserver-proxy
https://github.com/chimurai/http-proxy-middleware#context-matching

Sigh.
(In reply to Ed Morley [:emorley] from comment #0)
> * use `yarn start:local` which builds the UI but then watches/rebuilds as
> necessary

I'm finding in a local vagrant that `yarn start:local` does not appear to continue watching/rebuilding on its own, if I understand Ed's comment above correctly.

I am finding for each development edit, to see the result I have to manually delete my /dist, and do another `yarn start:local` to fully rebuild dist each time. Could be pilot error on my part though, if there's some trick I'm missing or isn't in the docs.
Yeah that's expected at the moment (but also unrelated to this bug), Cameron asked about the same a few weeks ago. I've added a todo to the general "fix the fallout from moving to a mandatory UI build" bug (bug 1353014 comment 12) to track this.
Bug 1395356 will make this easier + regardless we might as well wait until it lands first to avoid conflicts.
Depends on: 1395356
Attachment #8974446 - Flags: review?(cdawson)
Comment on attachment 8974446 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3534

Nothing short of fantastic!  :)
Attachment #8974446 - Flags: review?(cdawson) → review+

Comment 7

7 months ago
Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/55654a1195335b08b92a84e10f65dcc1ec73d5d3
Bug 1363722 - Allow login to work in all development workflows (#3534)

Previously it was not possible to test features that required an
authenticated user when:
* using `yarn start` with Vagrant (bug 1363722), which meant slower
  watch builds
* pointing the UI at the prod/stage API (bug 1317752), which was
  extremely limiting

Now login works in all environments, since the frontend no longer uses a
URL prefix, but instead webpack-dev-server proxies non-webpack URLs to
the chosen `BACKEND_DOMAIN` - avoiding cross-domain issues. Cookies are
rewritten to remove any `secure` directive (which is set on production),
so that they can still be read from HTTP localhost. The `Referer` has to
also be changed to stop Django's CSRF checks from rejecting request.

The slower "build into `dist` and watch" mode is therefore no longer
necessary, so `yarn start:local` instead invokes webpack-dev-server just
like `yarn start` - and the `local-watch.js` workaround has been
removed.

Support for the "publish to GitHub with hardcoded `SERVICE_DOMAIN`"
workflow has been dropped, since it was already rarely used and there is
no way to make it support login.

The API domain environment variable was renamed to `BACKEND_DOMAIN` to
avoid potential confusion given it no longer behaves the same as
`SERVICE_DOMAIN` used to.

NB: For full stack Vagrant workflows users must now connect to port
*5000* on localhost, not 8000.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.