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

Frontend prep for migration to nodes #2451

Closed

Conversation

psychedelicious
Copy link
Collaborator

Frontend prep for migration to nodes

Working on getting things ready.

Organize internal state

  • Separate out generation parameters from other options
  • Rename all ambiguously named parameters (e.g. seamless --> shouldTileX & shouldTileY, stuff like this)
  • Similarly, move magic value type parameters from the UI layer (like floats that disable at 0 and enable at any other value) to a translation layer

API

  • Decide how to handle HTTP requests (straight fetch, RTK createAsyncThunk, some other library?)
  • Write functions to shape internal state into graphs

Probably a lot of other stuff.

@psychedelicious
Copy link
Collaborator Author

@blessedcoolant @kasbah I'll be working from this branch/PR on prep for the migration to the new API.

My intention is to clean up internal state quite a bit by refactoring its shape, and changing how a lot of the "magic value" parameters are handled (e.g. move the magic into an API payload preprocessor kinda area).

Also, to decide how to make HTTP calls - leaning towards RTK's createAsyncThunk API.

Let me know if you want to participate or have any suggestions or feedback.

Also @kasbah check out this PR and branch to see how the new system will work (if you haven't already).

@blessedcoolant
Copy link
Collaborator

Sounds like a plan. I'll get in on it and help out once I'm back on the system in a few days. Provided nature doesn't mess things up.

Good to have you back.

@kasbah
Copy link
Contributor

kasbah commented Jan 29, 2023

I don't have time for a proper review right now but looks like a lot of renaming of things to more appropriate things :) 👍

@kasbah kasbah removed their request for review January 29, 2023 10:21
@Kyle0654
Copy link
Contributor

You may want to look at OpenAPI generator and see if any of the languages/frameworks it can generate are useful.

@psychedelicious
Copy link
Collaborator Author

Redux Toolkit provides a very nice layer over createAsyncThunk called RTK Query. It can do API generation from OpenAPI.

Need to do some testing with regard to runtime generation - getting this working well will be the tricky part, and is needed for automatic UI generation for nodes.

@psychedelicious psychedelicious force-pushed the feat/prep-for-nodes branch 2 times, most recently from e4dc2fd to 79bd16a Compare February 1, 2023 13:39
@psychedelicious
Copy link
Collaborator Author

I've set up husky to format w/ prettier and run eslint on each commit. Also, I've formatted and linted the frontend codebase.

Frontend contributors will need to yarn install if they work on this branch

@psychedelicious
Copy link
Collaborator Author

Added a bunch more code quality stuff:

  • After building check stats.html for a breakdown of the bundle
  • Added a babel plugin to fix lodash imports to be treeshakeable and enforce that
  • Tweaked a lot of the lint/prettier config

Added RTK's OpenAPI code generator so can generate the API client. Right now it's pointed at the swagger petstore demo API. It generates on yarn (dev|build|build-dev) or you can manually generate with yarn gen-api.

At this point the next step is actually to start migrating to the new API.

PS: It's possible I've borked the github actions to lint and format, will worry about it later.

@blessedcoolant
Copy link
Collaborator

It seems @lstein moved the directory of the frontend. Maybe rebase this before going ahead with more changes.

@Kyle0654
Copy link
Contributor

Kyle0654 commented Feb 3, 2023

Ack I thought no major refractors were going to happen until after diffusers 😣

@psychedelicious
Copy link
Collaborator Author

Yeah I saw - no worries I'll rearrange things in my pr

@psychedelicious psychedelicious force-pushed the feat/prep-for-nodes branch 5 times, most recently from 7c1b468 to 81e174b Compare February 4, 2023 01:06
@blessedcoolant
Copy link
Collaborator

Let me know when this is in a state for a test run.

psychedelicious and others added 4 commits February 4, 2023 17:43
`options` slice was huge and managed a mix of generation parameters and general app settings. It has been split up:

- Generation parameters are now in `generationSlice`.
- Postprocessing parameters are now in `postprocessingSlice`
- UI related things are now in `uiSlice`

There is probably more to be done, like `gallerySlice` perhaps should only manage internal gallery state, and not if the gallery is displayed.

Full-slice selectors have been made for each slice.

Other organisational tweaks.
- `eslint` and `prettier` configs
- `husky` to format and lint via pre-commit hook
- `babel-plugin-transform-imports` to treeshake `lodash` and other packages if needed

Lints and formats codebase.
Use Redux Toolkit Query's OpenAPI code generator to generate TS API.

Currently just pointing at the Swagger Petstore endpoint for testing.
@psychedelicious
Copy link
Collaborator Author

psychedelicious commented Feb 5, 2023

@blessedcoolant
Ok, have two minimal working examples.

  1. feat/prep-for-nodes using openapi-typescript-codegen to generate the API client. This provides minimal wrappers around fetch.
  2. feat/prep-for-nodes-rtk using @rtk-query/codegen-openapi to generate the API client. This provides more feature-rich wrappers around fetch and puts all data into redux for you, which I think is less useful than it sounds for us. It generates hooks for each API route which is pretty cool.

Both branches just have a couple packages different, and should provide the same UI, but use different generated APIs.

There are other options for API generation:

Set up

You will need another clone of InvokeAI to run the nodes server bc it isn't updated yet.

Now you should see a create and invoke button at the top of the app. It will say disconnected but the nodes stuff will still work. Create first then invoke, should make you a pizza and show it in the OS image viewer :)

@psychedelicious psychedelicious force-pushed the feat/prep-for-nodes branch 2 times, most recently from d9937e1 to 5e1a72e Compare February 5, 2023 10:25
@psychedelicious
Copy link
Collaborator Author

Closing. Will open a new PR for nodes migration when we get to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants