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

The road to Fresh 2.0 #2363

Open
marvinhagemeister opened this issue Mar 14, 2024 · 71 comments
Open

The road to Fresh 2.0 #2363

marvinhagemeister opened this issue Mar 14, 2024 · 71 comments

Comments

@marvinhagemeister
Copy link
Collaborator

marvinhagemeister commented Mar 14, 2024

The road to Fresh 2.0

tl;dr: Fresh will become massively simpler and be distributed via JSR with the upcoming 2.0 release. Express/Hono-like API and true async components and much more.


Back to Fresh!

Since yesterday, I've been going back to working on Fresh again. During the past few months I helped with shipping https://jsr.io/ which meant that Fresh sat on the back burner for me. Admittedly, I needed a bit a break from Fresh as well, and JSR came along at the right time.

Working on JSR allowed me to put myself into the user's seat and get first hands experiences with Fresh as a user, which was nice change! It also uncovered a lot of things where I feel we can make Fresh much better.

With JSR beeing out of the door, the next task for me is to move Fresh over from deno.land/x to JSR. Given that this is a bit of a breaking change, @lucacasonato and I were wondering what a potential Fresh 2.0 release could look like.

We've never done a breaking Fresh release before, but now, after having worked for nearly a year on Fresh, the timing feels right. It allows us to revisit all the ideas in Fresh and part ways with those that didn't pan out.

We spent yesterday morning going through the most common problems people shared on Discord and in our issue tracker to get a good picture of where Fresh is at. The overall theme we want to have for Fresh 2, is to be massively simpler than before.

A new plugin API

It became pretty clear that many issues relate to the "clunkyness" of the current plugin API. It's the most common way to extend Fresh itself with capabilities that it doesn't include out of the box. Whilst it received many cool new features over time, it quite never felt elegant as it should be. This is no surprise given that nothing in Fresh itself makes use of it.

Back in July of last year there was some explorations on making Fresh's API more express/Hono-like #1487 and yesterday we realized that this is the perfect solution for nearly all the problems encountered with the current plugin API. Here is what we're picturing it to look like:

const app = new FreshApp();

// Custom middlewares
app.use(ctx => {
  console.log(`Here is a cool request: ${ctx.url}`)
  return ctx.next();
});

// Also can be branched on by HTTP method
app.get(ctx => {
  return new Response("it works!")
});

// Adding an island (exact function signature yet to be determined)
app.addIsland(...)

// Finally, start the app
await app.listen();

A Fresh plugin would change from the complex object it is today, to being nothing more than a standard JavaScript function:

function disallowFoo(app: App) {
  let counter = 0;

  // Redirect to / whenever a route contains
  // the word "foo".
  app.use((ctx) => {
    if (ctx.url.pathname.includes("foo")) {
      counter++;
      return ctx.redirect("/");
    }

    return ctx.next();
  });

  // Add a route that shows how many times we
  // redirected folks
  app.get("/counter", () => {
    const msg = `Foo redirect counter: ${counter}`;
    return new Response(msg);
  });
}

// Usage
const app = new FreshApp();

// Adding the plugin is a standard function call
disallowFoo(app);

await app.listen();

The beauty about this kind of API is that many features in Fresh are a just standard middleware, and the more complex ones like our file system router and middleware handler would just call methods on app. The internals of Fresh will be implemented the exact same way as any other plugin or middleware. This eliminates the mismatch of the current plugin API and the Fresh internals of today.

Simpler middleware signature

The current middleware signature in Fresh has two function arguments:

type Middleware = (req: Request, ctx: FreshContext) => Promise<Response>;

In most of our code we noticed that we rarely need to access the Request object. So most of our middlewares just skip over it:

// We don't need _req, but still need to define it...
const foo = (_req: Request, ctx: FreshContext) => {
  // Do something here
  const exists = doSomething();
  if (!exists) {
    return ctx.renderNotFound();
  }

  // Respond
  return ctx.next();
};

It's a minor thing, but it's a bit annoying that you always have to sorta step over the req argument. With Fresh 2.0 we're planning to move it onto the context object.

// Fresh 1.x
const foo = (req: Request, ctx: FreshContext) => new Response("hello");

// Fresh 2.0
const foo = (ctx: FreshContext) => new Response("hello");

Same arguments for sync and async routes

Orignally, I've modelled async routes after the middleware signature. That's why they require two arguments:

export default async function Page(req: Request, ctx: RouteContext) {
  // ...
}

With the benefit of hindsight, I consider this a mistake. What naturally happens is that folks tend to start out with a synchronous route and add the async keyword to make it asynchronoues. But this doesn't work in Fresh 1.x.

// User starts out with a sync route
export default function Page(props: PageProps) {
  // ...
}

// Later they want to make it async, but this
// breaks in Fresh 1.x :S
export default async function Page(props: PageProps) {
  // ...
}

So yeah, we'll correct that. In Fresh 2.0 this will work as expected.

// Sync route same as in Fresh 1.x
export default function Page(props: PageProps) {
  // ...
}

// Async route in Fresh 2.0
export default async function Page(props: PageProps) {
  // ...
}

True async server-side components

In truth, the async route components in Fresh 1.x are a bit of a beautiful lie. Internally, they are not components, but a plain function that happens to return JSX. We take the returned JSX and pass it to Preact to render and that's all
there is to it. The downside of that approach is that hooks don't work inside the async funciton, because it's not executed in a component context. In fact async routes in Fresh 1.x are called way before the actual rendering starts.

// Fresh 1.x async route
export default async function Page(req: Request, ctx: RouteContext) {
  // This breaks, because this function is not a component in Fresh 1.x,
  // but rather treated as a function that happens to return JSX
  const value = useContext(MyContext);
  // ...
}

You might be wondering why we went with this approach and the main reason was that Preact didn't support rendering async components at that time. This was the quickest way to get something that gives you most of the benefits of async components with some tradeoffs into Fresh.

The good news is that Preact recently added support for rendering async components on the server itself. This means we can finally drop our workaround and support rendering async components natively.

// Fresh 2.x async components just work
export default async function Page(ctx: FreshContext) {
  // Works as expected in Fresh 2.0
  const value = useContext(MyContext);
  // ...
}

This isn't restricted to just route components either. With the exception of islands or components used inside islands, any component on the server can be async in Fresh 2. Components in islands cannot be async, because islands are also rendered in the browser and Preact does not support async components there. It's unlikely that it will in the near future either as rendering in the client is much more complex, given that it mostly has to deal with updates which are not a thing on the server.

Simpler error responses

Fresh 1.x allows you two define a _500.tsx and a _404.tsx template at the top of your routes folder. This allows you to render a different template when an error occurred. But this has a problem: What if you want to show an error page when a different status code other than 500 or 404 is thrown?

We could always add support for more error templates in Fresh, but ultimately, the heart of the issue is that Fresh does the branching instead of the developer using Fresh.

So in Fresh 2.0 both templates will be merged into one _error.tsx template.

# Fresh 1.x
routes/
   ├── _500.tsx
   ├── _404.tsx
   └── ...

# Fresh 2.0
routes/
   ├── _error.tsx
   └── ...

And then inside the _error.tsx template you can branch and render different content based on the error code if you desire:

export default function ErrorPage(ctx: FreshContext) {
  // Exact signature to be determined, but the status
  // code will live somewhere in "ctx"
  const status = ctx.error.status;

  if (status === 404) {
    return <h1>This is not the page you're looking for</h1>;
  } else {
    return <h1>Sorry - Some other error happend!</h1>;
  }
}

With only one error template to deal with, this makes it a lot easier to allow them to be put anywhere. So will be able to use different error templates for different parts of your app.

# Fresh 2.0
routes/
  ├── admin/
  │   ├── _error.tsx # different error page for admin routes
  │   └── ...
  │
  ├── _error.tsx # error template for everywhere else
  └── ...

Adding <head> elements from Handlers

Whilst not ready for the initial Fresh 2.0 release, we plan to get the guts of Fresh ready for streaming rendering where both the server and client can load data in parallel. Again, this will not be part of the initial 2.0 release, but might land later in the year. But before we can even explore that path, some changes in regards to how Fresh works are required.

With streaming the <head> portion of an HTML document will be flushed as early as possible to the browser. This breaks the current <Head> component which allows you to add additional elements into the <head>-tag from anywhere else in your component tree. By the team the <Head> component is called, the actual <head> of the document has long been flushed already to the browser. There is no remaining head on the server we could patch anymore.

So with Fresh 2.0 we'll remove the <Head> component in favour of a new way of adding elements from a route handler.

// Fresh 2.0
export const handlers = defineHandlers({
  GET(ctx) {
    const user = await loadUser();

    // Handlers in Fresh 2.0 allow you to return either
    // a `Response` instance, or a plain object like here
    return {
      // Will be passed to the component's `props.data`
      data: { name: user.name },
      // Pass additional elements to `<head>`
      head: [
        { title: "My cool Page" },
        { name: "description", content: "this is a really cool page!" },
        // ...
      ],
    };
  },
});

export default defineRoute<typeof handlers>(async (props) => {
  return <h1>User name: {props.data.name}</h1>;
});

EDIT: This API is the least fleshed out of the ones listed here and might change. The main goal we have is to get rid of the <Head> component.

Appendix

These are the breaking changes we have planned for Fresh 2. Despite some of them being breaking changes updating a Fresh 1.x project to Fresh 2 should be fairly smooth. Although this is quite a long list of bigger features, I spent yesterday and today hacking on it and got way further than I anticipated. Most of the features are already implemented in a branch, but lots of things are still in flux. It's lacking quite a bit of polish too. There will likely be some bigger changes landed in the coming weeks in the main branch.

It's too early yet to be tried out, but there will be an update soon. Once the actual release date approaches we'll also work on adding an extensive migration document. Given that it's early days and I just started back working on Fresh
yesterday, these things don't exist yet. The details of some of the features listed here and how they will be implemented might change, but I think the rough outline is pretty solid already.

I'm pretty excited about this release because it fixes many long standing issues and massively improves the overall API. Playing around with early prototypes it makes it much more fun to use too. I hope you are similarly excited about this release.

@CAYdenberg
Copy link
Contributor

If head is specified as JSON, rather than JSX, will it be possible to create arbitrary tags? I gather from your example that title: string will create a <title> and name: string; content: string will create a <meta>, but this doesn't seem very generic.

@fazil47
Copy link

fazil47 commented Mar 15, 2024

Are there any plans to use vite? Would be nice to fix #978 before 2.0 as well.

@marvinhagemeister
Copy link
Collaborator Author

@CAYdenberg The head API is the least fleshed out from the points. It should definitely be possible to set any tag you want.

@fazil47 No plans to switch to vite at the moment. Good point, #978 will be resolved before 2.0 .

@BrunoBernardino
Copy link

Are there any plans/ideas to support functions as props in islands, even if with some limitations? I personally find that the hardest thing to live with, using Fresh. It limits code re-usability considerably.

For example, it's not uncommon to build something like:

<ComplexOrBigComponent onClick={onComponentClick} /> inside an island and have the ComplexOrBigComponent.tsx simply call onClick from its props, but this doesn't work with Fresh because of the serialization for SSR.

I understand why it's done (because of the SSR), but there should be some way around it, even if it's disabling SSR for those components automatically, I believe, and it would make it a lot more easy to migrate existing code and patterns.

@marvinhagemeister
Copy link
Collaborator Author

marvinhagemeister commented Mar 18, 2024

No, that doesn't fit into the 2.0 release schedule. Serializing functions would require a complex machinery of tracking all accessed variables which is very error prone. I don't have an idea of how to solve that at the moment, so this requires further research before we can think of tackling it.

@BrunoBernardino
Copy link

BrunoBernardino commented Mar 18, 2024

My suggestion would be to not serialize that component, just let it be completely rendered in the client, as I don't think it would be possible to serialize any function reliably.

@CAYdenberg
Copy link
Contributor

If I'm understanding @BrunoBernardino 's point, the limitation he's experiencing is around code reusability. In other words, the ability to use a component as an island or not-an-island, depending on the context. In other words, for components that accept a function as an optional prop, it would be nice if there were a way to declare them as an island (or not-an-island) other than what folder they are in.

@BrunoBernardino
Copy link

BrunoBernardino commented Mar 18, 2024

@CAYdenberg that's exactly it! I'd like to create "dumb" components in components/ that receive functions as props from some islands, and wouldn't mind losing the island's benefits in those situations.

@CAYdenberg
Copy link
Contributor

Well, if it helps, components that are children of islands can receive functions as props. So, your ComplexOrBigComponent could live in components/, receive onClick when used as a descendent of an island, and not receive it when used outside of an island. Just mark onClick as being optional in your props interface declaration (and handle the cases where it is undefined).

@BrunoBernardino
Copy link

@CAYdenberg I need to try that, as that should solve my problem! I'll report back once I've had the chance to do so.

@miguelrk
Copy link

Nice points @BrunoBernardino @CAYdenberg ... also, I'd like to bring up proper support for HMR to the table, would that fit into 2.0?

@marvinhagemeister
Copy link
Collaborator Author

Proper HMR support in Fresh requires some changes in Deno CLI. It won't land as part of Fresh 2.0 but rather in the following release 2.1. That's the current plan.

@BrunoBernardino
Copy link

@CAYdenberg that totally solved my problem, thank you so much!

@KnorpelSenf
Copy link

Regarding the middleware, I have done an innovative step when I invented middleware trees for my framework. It's a much smoother thing than what express did, as it lets you combine middleware (and thus, plugins) in a much more flexible way.

If you feel like reading more about it, https://grammy.dev/advanced/middleware is the place to go. If, however, you don't want to take inspiration from that, please ignore me. Keep up the great work!

@lionel-rowe
Copy link
Contributor

Exciting stuff! Any ideas on projected release timeline yet?

@marvinhagemeister
Copy link
Collaborator Author

@KnorpelSenf thanks for sharing, that was interesting read! In our case we take routing into account which allows us to effectively flatten the tree into a flat list of middlewares + routes. This saves us a bit of book keeping and seems to work very well so far.

@lionel-rowe I'm making good progress at the moment and the hope for it is to have it out in a couple of weeks, unless I run into unforeseen challenges. I definitely want to get this into user's hands as soon as possible.

@KnorpelSenf
Copy link

KnorpelSenf commented Mar 27, 2024

@marvinhagemeister there's no book-keeping required for such an implementation. It's surprisingly trivial to do this, i.e. 10-20 lines of code (depending on how you count). We did it like this: https://github.com/grammyjs/grammY/blob/38efc3c7729eef7133446451bdd7056ad4b09004/src/composer.ts#L196

What I'm rather concerned about is that a nested middleware system prevents some performance optimizations. While you could technically implement fast routing per node, it isn't really possible to directly jump to the right handler deep down in the middleware tree. In my case, the functionality is worth that, but HTTP routing can be optimised a lot better, so the opportunity cost is different for fresh. You might not want to go with middleware trees because of this. I'll leave it up to you :)

@marvinhagemeister
Copy link
Collaborator Author

@KnorpelSenf The implementation you have there is quite elegant! I like it!

Agree about the concerns of nested middlewares potentially preventing performance optimizations. The way it currently works is that internally middlewares and routes are stored like this:

[
  * [...middlewares]
  GET / [...routeMiddlewares]
  GET /foo/bar [...routeMiddlewares]
  POST /api/foobar [...routeMiddlewares]
]

This means that an incoming request needs to do two steps:

  1. Grab the middleware array at the first index in the array, because those apply to all routes, if one of them returns a response we can bail out early
  2. Match one of the next route entries in the array. Once a route matches you already have the correct list of next middlewares to run, so no further checks are required.

Apart from the route matching, this system conceptually just walks over a flat array until one of the middlewares returns a response.

@aakashsigdel
Copy link

Great work and exciting times ahead. Are there any plans on supporting css modules, css in js and such with 2.0?

@marvinhagemeister
Copy link
Collaborator Author

@aakashsigdel CSS modules require transpilation and I've been getting pushback when proposing to transpile the server code in Fresh or landing CSS modules right in Deno itself. So no progress on that front. When it comes to CSS-in-JS something like styled-components will never be a got fit with Fresh as it turns every component into a client component effectively.

The good news though is that with 2.0 we're in a much better position to potentially add all those transpilation stuff, even for server code.

@Twitch0125
Copy link

Would it be possible to extract template rendering/islands into a plugin? It'd be awesome if you could mix/match frameworks similar to Astro, but with the ergonomics of Fresh

@marvinhagemeister
Copy link
Collaborator Author

@Twitch0125 The new architecture certainly makes this easier as rendering isn't spread out throughout the entire code base like it is in Fresh 1.x . That said there are no plans to support multiple frameworks at the moment. The worry is that supporting more than one framework requires more maintenance effort, likely more than we have the bandwidth for at the moment.

@fabianmendozaospina
Copy link

Fresh 2.0 promises to bring great improvements. When do you think it will be released?

@marvinhagemeister
Copy link
Collaborator Author

@FabianMendoza7 soon™

@janvhs
Copy link

janvhs commented Apr 8, 2024

Proper HMR support in Fresh requires some changes in Deno CLI. It won't land as part of Fresh 2.0 but rather in the following release 2.1. That's the current plan.

Without having read the Fresh source, yet. Would be great if you could somehow make this loosely connected to Fresh, as it could be really useful for developing SPAs with Preact, Deno and Esbuild.

@marvinhagemeister
Copy link
Collaborator Author

Proper HMR support in Fresh requires some changes in Deno CLI. It won't land as part of Fresh 2.0 but rather in the following release 2.1. That's the current plan.

Without having read the Fresh source, yet. Would be great if you could somehow make this loosely connected to Fresh, as it could be really useful for developing SPAs with Preact, Deno and Esbuild.

Agree, I've been pushing for solutions that are not specific to Fresh for that internally. Something that any developer using Deno would profit off, not just Fresh.

@dinfer
Copy link

dinfer commented Jun 15, 2024

Awesome! when can i use the 2.0 version?

@Xuhv
Copy link

Xuhv commented Jun 16, 2024

I think it is useful to specify islands freely so that we can place the islands component close to the route components where they are used, but alpha.16's fsRoutes only scan components in islands dir.

@marvinhagemeister
Copy link
Collaborator Author

marvinhagemeister commented Jun 16, 2024

@Xuhv That's already supported in both Fresh 1.x and 2.x . Any folder inside the routes/ directory that is named like this (_foo) will be ignored. If it's named like (_islands) then it will be treated as an additional island directory. This means you can do this:

  • routes/about/(_islands)/MyAboutIsland.tsx
  • routes/(_islands)/OtherIsland.tsx
  • ...

@tleperou
Copy link

I'd be happy to give a hand with the doc of Fresh 2; is there a thread to track or anything that help me get into it ?

@CAYdenberg
Copy link
Contributor

I would also like to help with docs and beta testing for 2.0 if the team is willing. 🔬

@timreichen
Copy link
Contributor

timreichen commented Jun 30, 2024

I am trying out @fresh/core@^2.0.0-alpha.18.
It seems that FreshContext uses req instead of request as a property name. I was wondering if it would make sense to use the full name request instead as js web apis normally do not abbreviate property names.

@csvn
Copy link
Contributor

csvn commented Jul 1, 2024

For comparison with a few other frameworks I could think of:

  • Express: Not comparable, as it's request object comes as a parameter and can be named anything (though the docs use req it seems)
  • Koa: Interestingly has both req and request. The former is used for Node native request object.
  • Hono: Uses req for it's context. Hono is very popular and modern, so feels like good match
  • Oak - Uses request for it's context. Oak is pretty much a modern Express-like substitute originally built for Deno

Personally, with such core and well recognized concepts that are a in frequent use, and with everyone's editor showing req: Request in code completions, I prefer req over request. It's a shared convention already, and it helps keep code more readable (reduces likelyhood that we must split one line into two). Especially since context.req (or ctx.req) will be used in (almost) every middleware function and custom route handler.

@timreichen
Copy link
Contributor

For comparison with a few other frameworks I could think of:

  • Express: Not comparable, as it's request object comes as a parameter and can be named anything (though the docs use req it seems)
  • Koa: Interestingly has both req and request. The former is used for Node native request object.
  • Hono: Uses req for it's context. Hono is very popular and modern, so feels like good match
  • Oak - Uses request for it's context. Oak is pretty much a modern Express-like substitute originally built for Deno

Thank you for the listing. I guess that shows though, that there is not a complete consensus about the usage of req.

Personally, with such core and well recognized concepts that are a in frequent use, and with everyone's editor showing req: Request in code completions, I prefer req over request. It's a shared convention already, and it helps keep code more readable (reduces likelyhood that we must split one line into two). Especially since context.req (or ctx.req) will be used in (almost) every middleware function and custom route handler.

Imo we should not design apis based on what would likely be best formatted but what is the most consistent with other apis. I think modern tools (autocompletion) and the js language (destructuring e.g. ({ request: req }) => ...) can handle such cases well enough if needed.

@rojvv
Copy link
Contributor

rojvv commented Jul 1, 2024

I advocate for request.

@marvinhagemeister
Copy link
Collaborator Author

Fresh itself always used req as the variable names in all of our docs, see: https://fresh.deno.dev/docs/concepts/middleware

export async function handler(
  req: Request,
  ctx: FreshContext<State>,
) {
  ctx.state.data = "myData";
  const resp = await ctx.next();
  resp.headers.set("server", "fresh server");
  return resp;
}

The code snippets on deno.com use that too:

Screenshot 2024-07-01 at 14 05 34

And on top of that this has been the convention in express, koa, hono and others. Changing that would make Fresh the odd one, which would be strange.

@timreichen
Copy link
Contributor

Fresh itself always used req as the variable names in all of our docs, see: https://fresh.deno.dev/docs/concepts/middleware

export async function handler(
  req: Request,
  ctx: FreshContext<State>,
) {
  ctx.state.data = "myData";
  const resp = await ctx.next();
  resp.headers.set("server", "fresh server");
  return resp;
}

Yes but variable names can be any name, it is not the same as a property name of an object.

And on top of that this has been the convention in express, koa, hono and others. Changing that would make Fresh the odd one, which would be strange.

I would argue that established web api names should be weighted more than framework conventions. FetchEvent also uses request.
I think Oak and Koa has it right and compared to that, Fresh would not be the odd one out.

@mayank99
Copy link
Contributor

mayank99 commented Jul 1, 2024

I do think it's a bit weird for a modern framework to use req in a brand new API. Property names should clearly describe the thing. I don't see a good reason to abbreviate it when IDE autocomplete and intellisense exist.

Previously, it was just a function argument and had an explicit type that fully spells it out, avoiding the ambiguity.

(req: Request) => {}

Whereas now, it's an actual property name and it may be destructured from the context object. Using req here almost looks like a mistake.

({ req }: FreshContext) => {}

Personally, I also think reduces readability because "req" is not a word. It would also not be announced correctly for developers who rely on screen-readers.

If we're looking at other frameworks, Astro, SvelteKit and Remix all use request as the property name. Next.js has a function param (similar to Fresh 1) but their examples name it request.

@csvn
Copy link
Contributor

csvn commented Jul 1, 2024

Thank you for the listing. I guess that shows though, that there is not a complete consensus about the usage of req.

There will never be consensus. Some teams avoid abbreviations, some don't. It also seems fairly even in a rough code search 🙂

  • 141k files for .req.
  • 227k files for .request.
    • This includes ~50k files of e.request. and event.request.

Imo we should not design apis based on what would likely be best formatted but what is the most consistent with other apis. I think modern tools (autocompletion) and the js language (destructuring e.g. ({ request: req }) => ...) can handle such cases well enough if needed.

I agree to a degree. I would not argument for making abbreviations to any property just to make it format better. In this case it's something that will already be encountered in the Fresh docs; req as abbreviation for Request.

It's not keystrokes I want to save, but being able to see and scan more code without "noise". Longer property/parameter names just makes it more likely one line can become 2-4 just to format nicely. It's often the stuff after that is more relevant too, e.g. .headers or .url, and these are often wrapped in getCookies() or new URL() to interact with. The req property is one of the most frequently used properties, so keeping it shorter is a big plus for me in terms of readable (not writable) code.

Yes but variable names can be any name, it is not the same as a property name of an object.

Even if parameters can be named anything, I think it's still relevant as it shows a shared convention in the ecosystem, both for library and application code.

I would argue that established web api names should be weighted more than framework conventions. FetchEvent also uses request.
I think Oak and Koa has it right and compared to that, Fresh would not be the odd one out.

I don't think what the platform does factors in heavily here; the example is just camelCased Request as property name. No one argues that the Request class should be named Req. Given how much in the ecosystem both req and ctx are used to represent Request and Context, this is more a style preference of the project writing the code. I.e. if abbreviations are allowed or not.

Deno and this project seems to lean into abbreviations as long as they easily deduced; req, res, ctx, Addr (remoteAddr, localAddr) and more. With documentation, types, and interfaces for every property, I feel it's even less of a need to explicitly write out .request when the type for the property explicitly shows it's an instance of Request.

@miguelrk
Copy link

miguelrk commented Jul 2, 2024

My two cents are:

in Fresh 1.x, since req was a handler argument, abbreviating it as a convention (one could also use request or others) was fine (just like the abbreviated ctx)

export const handler = (req: Request, ctx: FreshContext<State>) => {
  console.log(req);
}

in Fresh 2.x, since its no longer an argument, but a property of ctx, I also think ctx.request makes more sense (to also match its type Request). One can still use ctx, context or whatever, since its an argument, but a property should not be abbreviated in my opinion... (same would go for Addr instead of Address, I think a framework should not encourage abbreviations no matter how "well-established", since not everyone will be familiar with them, and the few keystrokes are just not worth it.

export const handler = (ctx: FreshContext<State>) => {
  console.log(ctx.request);
}

Not wishing to delay the 2.0 release, but if this change gonna happen, then I figured this is the right time for it to happen (still in time?).

@timreichen
Copy link
Contributor

List addition: Astro uses Astro.request and Svelte uses RequestEvent, therefore also request for routing.

@mayank99
Copy link
Contributor

mayank99 commented Jul 6, 2024

@timreichen Yes, I mentioned Astro, Svelte and Remix in my comment above.

@randreu28
Copy link

Apart from the req vs request drama, any updates on when will Fresh 2.0 will be released? Will the incoming release of Deno 2.0 be timed with the release of Fresh 2.0 aswell, or it has nothing to do with it and will be released separetly? Are there any specified dates?

@heyk1n
Copy link

heyk1n commented Sep 14, 2024

Apart from the req vs request drama, any updates on when will Fresh 2.0 will be released? Will the incoming release of Deno 2.0 be timed with the release of Fresh 2.0 aswell, or it has nothing to do with it and will be released separetly? Are there any specified dates?

no specific date, Marvin is currently focusing on the release of deno 2.0, but fresh 2.0 has released alpha version in jsr and i think it can be used for production

@nrako
Copy link

nrako commented Oct 10, 2024

Will Fresh v2 Plugin interface drops the ability to add Routes component ( aka PluginRoute) ?

I can see in the drafted API app.addIsland(...) but I don't see anything in the API nor in the released v2 alpha which would be equivalent to:

const plugin: PluginMiddleware = {
   routes: [
      { path: "no-leading-slash-here", component: SimpleRoute },
    ],

In the current main/canary (v2 alpha) I see app.island() but no interface of what was equivalent to the v1 Plugin interface, AFAICT.

Based on the new plugin interface, it seems this change is intentional, and I understand the goal of simplifying the API. I just want to ensure I fully grasp the direction here.

From what I gather, this approach might discourage plugins that enable certain routes meant to integrate directly within the app's layout. For example, a blog plugin configured to appear at /blog and fit more-or-less seamlessly into the app's existing layout. Instead, these "pluggable components" would need to be manually imported into the file system routes, and any centralised configuration or state would be accessed through ctx.state.somePluginConfig. Is that correct ?

@PierBover
Copy link

Now that Deno 2 has full Node compat, is there any chance to be able to use anything but Preact using Vite?

@csvn
Copy link
Contributor

csvn commented Oct 10, 2024

@nrako I have not tested this code, but I think that a plugin might be called with App as parameter, similar to '@fresh/plugin-tailwind' and fsRoutes, and then use the latter to add routes.

// @example/my-fresh-plugin/mod.ts
import { fsRoutes } from 'fresh';

export async function myFreshPlugin(app: App) {
  await fsRoutes(app, {
    dir: import.meta.dirname,
    loadRoute: (path) => import(`./routes/${path}`)
  });
}
// main.ts
import { App } from 'fresh';
import { myFreshPlugin } from 'jsr:@example/my-fresh-plugin';

const app = new App();
await myFreshPlugin (app);

The fsRoutes() method just adds routes via file system, so as long as there's a valid dir/loadRoute config I think a plugin should be able to add routes the same way as the "primary" app.

It's also possible that a plugin can return a App itself, and provide it so it can be mounted on a path:

import { App } from 'fresh';
import { appFreshPlugin } from 'jsr:@example/app-fresh-plugin';

const app = new App();

app.mountApp('/plugin-path', await appFreshPlugin());

Without docs, I'm guessing a bit. But to me, the base primitives we get from @fresh/core looks flexible enough for plugins to use how they see fit. Just passing in App and adding endpoints to it via .get(), .post(), .all() etc seems plenty flexible for most cases.

@marvinhagemeister
Copy link
Collaborator Author

marvinhagemeister commented Oct 10, 2024

I think I'm coming around to adding some sort of layout API everywhere instead of moving that off to its own plugin. There are lots of good use cases where you want a plugin route to integrate into an existing layout which can be done right now, but is a bit low-level. Originally, I had pictured a world where layouts would be just plain components, so you'd do:

app.get("/foo", ctx => {
  return ctx.render((
    <App>
      <Layout>
        <h1>this is a heading inside a layout</h1>
      </Layout>
    </App>
  );
});

Whilst components are composable on its own, it gets complicated when you're trying to compose routes from plugins. That's where the current API falls a bit short. Having something like app.layout() or another way to attach a layout to a route would fill that gap. I got this wrong and when I'm back working on Fresh I'm hoping to address this.

@nrako
Copy link

nrako commented Oct 10, 2024

@marvinhagemeister, @csvn, thanks for sharing your insights!

There are lots of good use cases where you want a plugin route to integrate into an existing layout which can be done right now

Exactly, this is indeed possible in v1.

I understand and can anticipate the complexity that arises when trying to compose routes using plugins. The simplicity of a Hono & express style API for the v2 Plugin API approach, has a lot of merits. It could indeed be the right direction, even if it involves a breaking change—it might prove better for the long-term. That’s the key consideration I was trying to understand.

But indeed, the ability to add routes at any path through a plugin while inheriting layouts from the app's file system (fsRoutes) can indeed be very useful in Fresh 🍋.

Having something like app.layout() or another way to attach a layout to a route would fill that gap.

However, I'm not totally getting how app.layout() would be used and fill that gap.

To clarify my thoughts, let’s imagine an application with the following file system structure and plugin usage:

routes/
  ├─  _app.tsx
  └─  team/
      ├─  _layout.tsx   // Team-specific layout
      └─  index.tsx     // Team home page
// main.ts
freshBlogPlugin(app, { path: '/team/blog' })

For the plugin API I could imagine something like that:

app.get(
  options.path, // "/team/blog", 
  ctx => {
    return ctx.render(
       <p>this is inside the app wrapper and the layouts, thus '_app.tsx' and 'team/_layout.tsx' </p>,
       {
           skipAppWrapper: false,
           skipInheritedLayouts: false,
        }
    );
  }
);

@marvinhagemeister
Copy link
Collaborator Author

Yup, something like that would be great. It would require the app instance to be made aware of the concept layouts and the app wrapper template. You'd still need a way to register layouts as probably not all layout files sit in the local project directory.

@nrako
Copy link

nrako commented Oct 10, 2024

You'd still need a way to register layouts as probably not all layout files sit in the local project directory.

That might be true. However, since this feature doesn’t seem to be available in v1, I’m not entirely sure about its necessity. I can’t think of a scenario where, as a Fresh user, I would need a plugin to inject a layout at a specific path.

@cdaringe
Copy link

cdaringe commented Oct 26, 2024

fixing async components

Will async components block the response stream or render tree? I presumed an async component would insert a placeholder or something in the html that, on settling of the component render, would transfer that html back into the placeholder on hydration, browser side.

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