netzo / fresh-netzo

Full-stack Deno Fresh meta-framework for building business web apps like internal tools, dashboards, admin panels and automated workflows.
https://netzo.io
MIT License
51 stars 3 forks source link

[plugins] refactor framework setup into fresh plugins #92

Closed miguelrk closed 8 months ago

miguelrk commented 9 months ago

TLDR: refactor the current project setup with a Netzo() entrypoint function in netzo.ts into the regular fresh project structure, with a regular netzo() fresh plugin to mount everything.

Context

In the spirit of aligning more to fresh, it is best to return to the default fresh project structure with a dev.ts, main.ts and a netzo.config.ts (extends the regular fresh.config.ts). We actually had it this way before, but we abstracted it away into a single netzo.ts for developer convenience (simpler project structure) and greater control (e.g. enforcing certain plugins). Going back to a netzo.config.ts would align netzo more with fresh, without sacrificing much DX. To do so, I see 3 alternatives:

1) Separate plugins This has the least DX, but more modularity, encouraging the plugin ecosystem to grow. Downside is that there is innevitable dependency between certain netzo plugins (by design, to ease the setup, e.g. components depend on unocss being enabled) and this modularity decreases the ability to enforce certain plugins depending on others. That being said, explicit is always better than implicity, and this could be solved easily by simply documenting it.

import { auth } from "netzo/core/auth/mod.ts"
import { api } from "netzo/core/api/mod.ts"
import { unocss } from "netzo/core/unocss/mod.ts"
export default defineConfig({
  plugins: [
    auth({...}),
    api({...}),
    unocss({...}),
  ]
});

2) Separate plugins (with barrell file) Same as above, with an additional barrel file import for DX convenience (despite perf issues with barrell files, this is opt-in so it's ok to offer it).

import * as netzo from "netzo/core/mod.ts"
export default defineConfig({
  plugins: [
    netzo.auth({...}),
    netzo.api({...}),
    netzo.unocss({...}),
  ]
});

3) Single plugin This allows enforcing certain functionalities required by netzo (e.g. platform features like auto-loading remote environmnet variables) and also since some modules depend on others (e.g. components depend on unocss being enabled).

import * as netzo from "netzo/core/mod.ts"
export default defineConfig({
  plugins: [
    netzo({
      auth: {...},
      api: {...},
      unocss: {...},
    })
  ]
});

Related

Additionally, simplifying the project structure and fresh server should ideally be done upstream in fresh-land.

miguelrk commented 9 months ago

Closing in favor of keeping Netzo() entrypoint function after thoughtful considerations when merging https://github.com/netzo/netzo/pull/93

miguelrk commented 9 months ago

From here:

Due to denoland/fresh@main/src/dev/cli.ts wanting a config. If we're planning on moving towards a more fresh-centric approach, it's probably necessary to have the file at some point, so might as well add it now.

I'd like to reopen this discussion @deer. I wasn't aware that fresh's cli task required the manifest. We definitively want to move towards a more fresh-centric approach. I gave it a go in this PR and landed on a sort of middleground. See my reasoning above.

As I mention there, I really like the idea of having a (sort-of) singleton pattern via export const netzo = Netzo({...});, since Netzo will keep growing in modules, and having both setup via Netzo({...}) and usage via netzo (in a single place) is best for netzo's DX.

Having said that, and since using netzo.config.ts would not allow using the original denoland/fresh@main/src/dev/cli.ts task, we could maybe add our own cli task that uses fresh's internal manifest() function, and simply uses netzo.ts to pick up the config from netzo.config:

maybe the following alternative is a good compromise, keeping the current netzo.ts entrypoint:

// netzo.ts: 
import { Netzo } from "netzo/mod.ts";

export const netzo = await Netzo({...});

if (import.meta.main) netzo.start();

we could add an internal (not exposed via --help) manifest subcommand to the netzo CLI:

// netzo/lib/cli/subcommands/manifest.ts
import { join, toFileUrl } from "../../../deps/std/path/mod.ts";
import { manifest } from "../../../deps/$fresh/src/dev/manifest.ts";
import { type FreshConfig } from "../../../deps/$fresh/server/mod.ts";

const args = Deno.args;

switch (args[0]) {
  case "manifest": {
    if (args[1]) {
      const CONFIG_TS_PATH = join(args[1], "netzo.ts");
      const config: FreshConfig = (await import(CONFIG_TS_PATH)).netzo.config;
      await manifest(args[1], config?.router?.ignoreFilePattern);
    } else {
      console.error("Missing input for manifest command");
      Deno.exit(1);
    }
    break;
  }
  default: {
    console.error("Invalid command");
    Deno.exit(1);
  }
}

What do you think of this approach? Since the task really only uses fresh's original manifest() function, I don't believe it makes us stray from our fresh-centric goal. I see this cli task as more of a utility which would be fine to adapt to netzo.

deer commented 9 months ago

I can also open a PR in Fresh to modify the CLI to accept the config file name via an argument. That way netzo.config.ts could have the same shape as fresh.config.ts, but with a few extra options. Then we can leverage what's already there, without duplicating code in netzo.

miguelrk commented 9 months ago

I agree, having that option from the fresh cli task would be best. I was already working on a PR for this, so I'll leave it for now and hope the fresh PR gets merged soon

deer commented 9 months ago

Well, there are no PRs getting merged anytime soon in Fresh, because Marvin is busy with JSR. So perhaps a two-pronged approach here:

  1. you do whatever you think is necessary in Netzo in the short term
  2. I'll modify Fresh to support the long term vision. When that's merged we can uptake it in Netzo.
miguelrk commented 9 months ago

Alright, thanks for the heads-up, that's good to know...

I'll proceed with 1 then: by vendoring that task and adjusting it for netzo rather than adding it as an (internal) cli subcommand 👍🏼.

deer commented 9 months ago

@miguelrk, please remind me of the long term goal here. I wanted to modify Fresh to support what we discussed here, but now I see that there isn't even a netzo.config.ts in either of the templates. Do you still want to introduce such a file? If not, I don't see how I can modify the Fresh CLI to support this.

miguelrk commented 9 months ago

Sure @deer, I did pursue this in #93 but dropped it due to the following reasonsing.

I'm still not entirely convinced though, and very open to discuss this. Ideally we'd have both things: alignment with fresh but keeping the simplicity in the setup that Netzo() gives (as the returned netzo instance bundles everything together, which is very practical).

An approach I also explored was:

// netzo.ts
const netzo = await Netzo({...})

and then import netzo in the original dev.ts and main.ts files and pass netzo.config: FreshConfig to their start functions.

But I didn't really see the benefit over

// netzo.ts
const netzo = await Netzo({...})

if (import.meta.main) netzo.start()

which requires 2 files less. I sometime likento think of this as a a precursor to this initiative, before it lands upstream in fresh. Also, NetzoConfig extends FreshConfig, so the entire Fresh options are still exposed if needed, so its not much of a "missalignment" with fresh in reality.

deer commented 9 months ago

Ok, thanks for your thoughts. I'll hold off on any modifications to Fresh until my understanding of Netzo radically increases. I still see the end goal as just using Fresh with a single Netzo plugin, but I'm not familiar enough with the situation yet in order to pursue that. If I figure it out, I'll let you know.