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

[Fastify] Extract logic into redwood plugin #8119

Merged
merged 44 commits into from
May 12, 2023
Merged

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Apr 25, 2023

Update: this one's experimental and it works so calling it good enough for now but there's some DX things I'd certainly like to do before it goes stable. If checks pass, getting into canary so others can be unblocked on dependent features.


This PR takes a stab at extracting out the fastify logic into a plugin. Spun off from the Container working group. Just a work in progress right now. Cc @Josh-Walker-GM


Motivation
We wish to reach a point where a user can avoid invoking the CLI to run api or web servers. This is pretty much a necessary step to reduce the docker image size.

Changes
A new package @redwoodjs/fastify which includes two fastify plugins for running the redwood api and web servers.

The cli package has been updated to have it's own command handlers for yarn rw serve rather than import the ones from api-server. These command handlers are similar but register the new web/api fastify plugins. There remains support for loading config from the server.config.js file.

This should not be breaking as there have been no changes to the api-server package. Those who use that package for their own purposes will have no changes. I would consider this PR a deprecation of the api-server package - this should be discussed with the team alongside the next steps below. The new cli handlers also support all the same arguments so there is no breaking change there.

Next steps

  1. Add an ejection command to scaffold a server.js|ts file in src/api and update cli command handlers to simply run that file if it exists. That change should remove support for the server.config.js file.
  2. Consider the same approach for the web side given future need for a web server. (Another PR)

@jtoar jtoar added the release:feature This PR introduces a new feature label Apr 25, 2023
@replay-io
Copy link

replay-io bot commented Apr 25, 2023

14 replays were recorded for b196757.

image 3 Failed
    useAuth hook, auth redirects checks
          ```
          locator.waitFor: Timeout 5000ms exceeded.
          =========================== logs ===========================
          waiting for locator('text=Username `[email protected]` already in use') to be visible
          ============================================================
          ```
        </ol>
      </details>
      <details>
        <summary><a href=https://app.replay.io/recording/7d74427a-a68b-4605-a5b9-16d5b70570a2>RBAC: Should not be able to delete contact as non-admin user</a></summary>
        <ol type="1">
          
          ```
          locator.waitFor: Timeout 5000ms exceeded.
          =========================== logs ===========================
          waiting for locator('text=Username `[email protected]` already in use') to be visible
          ============================================================
          ```
        </ol>
      </details>
      <details>
        <summary><a href=https://app.replay.io/recording/26236dab-1974-410c-b9c9-6736992f047a>Smoke test with dev server</a></summary>
        <ol type="1">
          
          ```
          page.textContent: Target closed
          =========================== logs ===========================
          waiting for locator('text=Meh waistcoat succulents umami')
          ============================================================
          ```
        </ol>
      </details>
      
image 11 Passed

View test run on Replay ↗︎

@Josh-Walker-GM
Copy link
Collaborator

Created https://community.redwoodjs.com/t/fastify-server-file-experiment/4851 which is empty right now but can be updated when things crystallise a little more.

@jtoar jtoar marked this pull request as ready for review May 12, 2023 17:28
@jtoar jtoar merged commit c02ae13 into main May 12, 2023
@jtoar jtoar deleted the ds-fastify/make-plugin branch May 12, 2023 18:58
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone May 12, 2023
jtoar added a commit that referenced this pull request May 12, 2023
* wip

* Fix apiRootPath option and add back some console logs

* 1 - make it work

* Add back support for server.config.js file, rework cli handling

* fix cli deps

* Revert changes to api-server package

* Remove cli dep on api-server

* add exp setup server

* rename to server file

* add toml step

* export coerceRootPath from @rwjs/fastify

* comment addApiPackages back in

* update template to include dotenv-defaults

env vars like DATABASE_URL won't be found without this code

* Add forum link

* Make rw serve run server.js|ts

* Prevent tests triggering experimental behaviour

* Fix windows paths in test case

* Restore createFastifyInstance in fastify package

* ignore server.config.js when using server file

* use web port in template

* yarn node instead of node

* Change server.config.js ignore warning

* Updated to use exp utils

* fix dep

* move to setup-server-file

* update readme

* update build

* fix deps

* fix import

* fix serve command

* style

* style

* fix test

* style

* style

* style

* style

* style

---------

Co-authored-by: Josh-Walker-GM <[email protected]>
@jtoar jtoar modified the milestones: next-release, v5.2.0 May 17, 2023
@jtoar jtoar mentioned this pull request Jan 21, 2024
8 tasks
jtoar added a commit that referenced this pull request Jan 21, 2024
This PR brings the server file out of experimental by implementing a
`createServer` function. This is a continuation of the work started in
#8119.

This API was designed in response to the feedback to #8119, which gave
users as much control as possible by more or less ejecting the code in
api-server. This resulted in users managing lot of code that really
wasn't their concern. In general it didn't feel like the Redwood way.
The new API still gives users control over how the server starts but
encapsulates the low-level details.

I've tried to make this PR as complete as possible. I feel like it's
reached that state, but there's still a few things I'd like to do. In
general I'd like to deduplicate all the repeated server code.

- [x] bring the server file out of experimental
- [x] type the `start` function
- [x] figure out how to handle the graphql function 
- [x] double check that `yarn rw dev` works well (namely, the watching)
- [x] double check that you can pass CLI args in dev and serve
- [x] the `yarn rw serve` command needs start two processes instead of
one with the server file
- [x] double check that env vars are being loaded
- [x] right now this is imported from `@redwoodojs/api-server`. long
term i don't think this is the best place for it

---------

Co-authored-by: Tobbe Lundberg <[email protected]>
Co-authored-by: Josh GM Walker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants