InvokeAI icon indicating copy to clipboard operation
InvokeAI copied to clipboard

[enhancement]: improve dev experience by building frontend in CI

Open ebr opened this issue 2 years ago • 0 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Contact Details

No response

What should this feature add?

Summarizing the Discord conversations:

  • Multiple streams of work on the frontend result in conflicting commits in invokeai/frontend/dist. The resolution path is to check out the PR branch, delete the dist, merge main, push the resolution commit, and hope no one else has merged any other commits to the same files into main in the meantime. This is tedious and burdensome.
  • "lightweight" contributions, such as language translations, also require a rebuild of the front-end. But contributors of such changes can not be expected to do frontend builds and maintain their branches with this degree of complexity.
  • frontend/dist is, for all intents and purposes, a build artifact. it should not be tracked by Git.

We should remove this directory from SCM and add workflows that build it in CI. This will solve dev experience issues described above, but will present a new set of challenges:

  • currently, non-frontend contributions do not have to touch frontend files at all; a pip install -e . is sufficient to get a working installation. With this change implemented, non-frontend contributors will have to install the Nodejs/yarn toolchain and perform front-end builds in order to test their changes in a local environment. They will also have to be aware of upstream changes to front-end code, and rebuild their local assets as needed.
  • package, container, and installer builds rely on the dist existing in source control.

The PR #2646 contains the beginnings of this work, but more work will be needed to ensure a clean transition:

  • move frontend/dist elsewhere in the repo, because it's not possible to make Git completely "forget" it (easily), since it's already tracked. (git filter-branch is not an option as it rewrites history; this ship has sailed)
  • add a workflow for the frontend build, which:
    • runs first in the workflow order;
    • outputs dist as a build artifact used by downstream jobs, such as build of the .whl and push to PyPi
  • update contributor documentation; ensure node/yarn is clearly described as a development prerequisite, with installation instructions;
  • add the front-end build to the container image
  • build installers in CI so that local build of the installer doesn't also require building the front-end. Workflows should be executed in the following order: frontend>wheel>installer. Each workflow should use the output artifact of preceding workflow to avoid re-work.

Alternatives

An alternative approach would be to allow CI to commit dist to the main branch (and prevent contributors from doing so). But allowing CI to make commits isn't usually desirable, is moderately complex to set up (as we have branch protections), and doesn't fully prevent anyone from causing conflicts by committing and pushing the frontend dist to a PR.

Aditional Content

This issue should be considered in the context of migration to nodes, because that implements API generation at runtime, and has impact on how we will build front-end going forward. This issue might become obsolete or change significantly based on that work.

ebr avatar Feb 14 '23 04:02 ebr