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

Initial design, prerequisites #1

Open
mcollina opened this issue Jan 27, 2023 · 29 comments
Open

Initial design, prerequisites #1

mcollina opened this issue Jan 27, 2023 · 29 comments

Comments

@mcollina
Copy link
Owner

We should verify there are all the pieces in Astro and Fastify to make this possible, and see how they can interact.

Something that would work extremely well from a Fastify perspective would be something like:

import fp from 'fastify-plugin'
import fastifyStatic from '@fastify/static'

export default async function (app, opts) {
  const { routes, dir } = await buildAstro(opts)
  
  // dynamic routes
  for (const route of routes) {
    app.get(route.path, (req, reply) => {
      const { result, statusCode, headers } = await route.serve()
      reply.code(statusCode).headers(headers)
      return result
    })
  }

  app.register(fastifyStatic, { dir, wildcard: false })
}

This approach would be extremely good from a production standpoint. However I'm not exactly sure how to fit an HMR model in here.

@matthewp
Copy link

This sounds great! Thanks a lot, having an API idea really helps me visualize what this all may look like. A lot of the pieces suggested here already exist, just not in the nice API forms being shown.

So I can either do some work on cleaning that API, and probably having something like a beta/preview npm tag so it can be used, or we could just import the internals and use those. I'll have to take a look and see what's more plausible.

I don't think the dev standpoint would be much harder. I actually think production might be slightly harder because I don't think you'll want to run the build when your production server starts up, instead you'd want to run the build in a CI, save the route information to a file somewhere, and then pull that in and use it. I'll think a bit more on this.

I wouldn't expect us to be able to get both development and production done on a stream, but we could try and see how far we get.

@mcollina
Copy link
Owner Author

I think the best approach would be to develop on top of the internals, and then extract public APIs from it.

@matthewp
Copy link

matthewp commented Feb 9, 2023

Ok, this is the version using internals:

//import fp from 'fastify-plugin'
import fastifyStatic from '@fastify/static';
import { fileURLToPath } from 'node:url';
import { createContainer } from './node_modules/astro/dist/core/dev/index.js';
import { createViteLoader } from './node_modules/astro/dist/core/module-loader/index.js';
import { createRouteManifest } from './node_modules/astro/dist/core/routing/index.js';
import { createDevelopmentEnvironment, renderPage, preload } from './node_modules/astro/dist/core/render/dev/index.js';

export default async function (app, { root }) {
  const container = await createContainer({ root });
  const loader = createViteLoader(container.viteServer);
  const manifest = createRouteManifest({ settings: container.settings }, container.logging);
  const env = createDevelopmentEnvironment(container.settings, container.logging, loader);
  
  // dynamic routes
  for (const route of manifest.routes) {
    app.get(route.route, async (req, reply) => {
      const filePath = new URL(route.component, root);
      const request = new Request(`http://localhost${req.url}`, {
        method: req.method
      });
      const response = await renderPage({
        env,
        filePath,
        preload: await preload({ env, filePath }),
        pathname: req.url,
        request,
        route
      });

      reply.code(response.status).headers(Object.fromEntries(response.headers.entries()));

      const body = await response.text();
      reply.send(body);
    })
  }

  app.register(fastifyStatic, {
    root: fileURLToPath(container.settings.config.publicDir),
    wildcard: false
  })
}

As you can see, this uses a lot of internals! And most of these things are not part of our "exports" map, so it can't be imported the normal way.

Luckily for production we have a higher-level abstraction. A class called App which is the thing that my astro-fastify adapter (and all of our production adapters) use.

I think it makes sense to create a subclass of that for development mode purposes. I'll work on that and then see how it cleans up this code and share it with you.

@matthewp
Copy link

matthewp commented Feb 9, 2023

One thing, I couldn't get streaming to work. Astro uses the whatwg request and response objects. I think for streaming I'll need to convert the web stream to a node:stream, is that right?

edit: I think Readable.fromWeb(response.body) should work.

@mcollina
Copy link
Owner Author

This makes 100% total sense. How do you see us plugging into this?

We can:

  1. make me do a PR to Astro and work together on that (code lives in your repo, you don't have anything else to expose)
  2. we add some of those to public astro APIs to get this flow working.

In the case of 2, there is no real need to @fastify/static if you have a way to serve files from within astro, or you can just expose the publicDir as a path via a decorator, telling people to serve it themselves.

@matthewp
Copy link

I think (2). We have external APIs for adapters and dev mode could work very similarly. I've started a PR here: withastro/astro#6231

Note that this PR is a draft and might take some time to finish up and get approval. But I've created a preview release tag we can play around with: astro@next--devapp

I'm working on an updated version of fastify-astro that uses this API. It should be good enough for the stream.

@mcollina
Copy link
Owner Author

awesome!

@matthewp
Copy link

Here's a version that uses the new DevApp I'm proposing:

import { DevApp } from 'astro/app/dev';

export default async function (fastify, { root }) {
  const app = new DevApp({ root });
  await app.load();
  
  // dynamic routes
  for (const route of app.routes) {
    fastify.get(route.route, async (req, reply) => {
      const request = new Request(`http://localhost${req.url}`, {
        method: req.method
      });

      const response = await app.render(request);      

      reply.code(response.status).headers(Object.fromEntries(response.headers.entries()));
      for await (const chunk of response.body) {
        reply.raw.write(chunk);
      }
      reply.raw.end();
    })
  }
}

This contains some bugs / incompletions we can tackle on the stream.

Here's the branches I have:

@matthewp
Copy link

@ematipico and I have been talking about picking back up this idea!

@mcollina
Copy link
Owner Author

How can I help? This would be amazing!

@ematipico
Copy link

I am not very familiar with the fastify APIs/plugin system. I know that fastify now has an official @fastify/vite plugin for vite. Can Astro, as fastify plugin, use this plugin under the hoods to spin a vite instance and eventually render a route?

@mcollina
Copy link
Owner Author

Astro assumes to be in control of the http server, same as Fastify. Currently you can wrap a Fastify instance inside an Astro instance, while I would like to do the opposite.

@fastify/vite can help in the sense that it provides a way to hook Vite up. Can Astro be spin up as a config of Vite?

@PierBover
Copy link

Can Astro, as fastify plugin, use this plugin under the hoods to spin a vite instance and eventually render a route?

This should only happen during dev, no?

Vite should not be used in production. In part because Vite can build the Node modules for rendering in the server (and obviously the client stuff too) but also because of this on-going issue with memory:

vitejs/vite#2433

@mcollina
Copy link
Owner Author

I think Vite is needed in production to do SSR.

@PierBover
Copy link

PierBover commented Nov 16, 2023

I think Vite is needed in production to do SSR.

It's not needed. When bulding, Vite creates the Node modules that will do the SSR.

I made this demo with Fastify and Svelte. This file is what does the Vite magic of loading .svelte files for SSR and when building it's converted into a regular Node file that imports Node modules without needing Vite.

Edit:

Here are the Vite docs for building for SSR.

https://vitejs.dev/guide/ssr#building-for-production

I'm sure @matthewp knows all about this though!

@ematipico
Copy link

ematipico commented Nov 16, 2023

Can Astro, as fastify plugin, use this plugin under the hoods to spin a vite instance and eventually render a route?

This should only happen during dev, no?

Yes, exactly, this would happen only in dev.

Vite should not be used in production. In part because Vite can build the Node modules for rendering in the server (and obviously the client stuff too) but also because of this on-going issue with memory:

vitejs/vite#2433

Agree. In production, Astro doesn't need to use Vite, although it requires a build step (using Vite) to compile everything to JS (for SSG or SSR).

@mcollina
Copy link
Owner Author

What's the interface of Astro in production?

@matthewp
Copy link

matthewp commented Nov 21, 2023

You have to write an adapter which is basically just a way to customize the built module's exports so that it can be used in production. Here's some pseudo-code:

adapter.js

import { NodeApp } from 'astro/app/node';

export function createExports(manifest, options) {
  const app = new NodeApp(manifest);
  return {
    app
  }
}

which will produce something like this:

dist/server/entry.mjs

const _exports = createExports(manifest);

export const app = _exports.app;

Which you can then use like so:

import { app } from './dist/server/entry.mjs';

const response = app.render(request);
// request can be a http.IncomingMessage or a whatwg Request
// response is a whatwg Response

Note that this example is exposing the NodeApp instance but an adapter can define whatever exports it wants to. This is designed this way for hosts that expect certain module exports (for example AWS Lambda expects a function named handler.

@mcollina
Copy link
Owner Author

This might work to integrate. Will take a look and see if we can have a prod experience.

@matthewp
Copy link

@ematipico For dev-mode we can use this to prevent Vite from starting its dev server: https://vitejs.dev/guide/ssr.html#setting-up-the-dev-server

@ematipico
Copy link

@ematipico For dev-mode we can use this to prevent Vite from starting its dev server: vitejs.dev/guide/ssr.html#setting-up-the-dev-server

Oh, that's neat! That should work very well for us

@mcollina
Copy link
Owner Author

Is there a way to check if a route is served by Astro?

@ematipico
Copy link

ematipico commented Nov 22, 2023

Yes, the class NodeApp exposes a public method called match that returns true if it knows the route. And you call like this: app.match('/blog')

@matthewp
Copy link

I assume @mcollina wants to define all the routes up front, right? We can likely expose that information in some way.

@mcollina
Copy link
Owner Author

If there is a routes table available that would be fantastic.
I can likely make do with the app.match() at the cost of a bit of overhead.

@rmcarias
Copy link

Thank you @matthewp and @mcollina for this update. This will be amazing to have (and am itching to use it when ready) as part of the Fastify suite!

@patheticGeek
Copy link

Any updates on this so far? seems like this has not been worked on for a while

@ematipico
Copy link

This week, we will release the Container API as experimental. I think this API is going to be very useful for Fastify, eventually.

@mcollina
Copy link
Owner Author

@matthewp @ematipico are you up for a pairing session where we try to assemble this module once that's release? Would be great to do a stream on this.

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

No branches or pull requests

6 participants