denoland / fresh

The next-gen web framework.
https://fresh.deno.dev
MIT License
12.26k stars 629 forks source link

proposal: add Deno KV OAuth plugin to Fresh #1733

Closed iuioiua closed 1 year ago

iuioiua commented 1 year ago

Deno KV OAuth now has a Fresh plugin. In the PR, there was the idea of having the plugin be located within the Fresh codebase instead of the Deno KV OAuth codebase. I imagine it looks something like this:

// main.ts
import { start } from "$fresh/server.ts";
import { createGitHubOAuth2Client, kvOAuthPlugin } from "$fresh/plugins/deno_kv_oauth.ts";
import manifest from "./fresh.gen.ts";

await start(manifest, {
  plugins: [
    kvOAuthPlugin(createGitHubOAuth2Client()),
  ],
});

I am for the idea, but my opinion is not strong. Keen to hear other opinions. CC @jollytoad, @mbhrznr

mbhrznr commented 1 year ago

personally i love the idea. afaik there's also the uno-css plugin in the pipeline and about to be released soon-ish.

@deer also worked on a bunch of plugins which would be great candidates.

just wondering where we'd draw the line on "official" plugins? but i'd definitely see it on this one in particular!

mitchwadair commented 1 year ago

Not sure my opinion matters too much here, but IMO it is a bit of a slippery slope to include plugins with Fresh itself. I think there should be some kind of line drawn as to what is "acceptable" to be included to prevent bloat and to prevent increasing the maintenance load on the Fresh team.

For this case, with Fresh and Deno KV OAuth being sort-of 1st-party libs for Deno, it would make sense to me to include a plugin for Deno KV OAuth. For third-party libraries, I would personally caution including plugins directly in the source for Fresh. I feel that including a README or wiki section with some available plugins for third party libraries may be a good alternative to directly including them in the source. Like an "awesome Fresh plugins" type thing

All that said, my opinion may differ from what the general philosophy for the Fresh team is regarding this sort of thing and that's okay. Just my $0.02 :)

iuioiua commented 1 year ago

I agree. There should be a well-defined criteria for plugins that are included and perhaps a 3rd party plugin directory.

jollytoad commented 1 year ago

I think it's important for the Deno community to detach itself from the node-way of thinking when it comes to libraries. They are not the same beasts as node packages.

There is no overhead on the user if plugins are imported directly from their own module URLs. The unused plugins or their dependencies never get downloaded like they would in a node package. It's not bloat in the same way as how node packages can bloat. The Deno std lib, for example, is fairly large, but it doesn't matter because you only ever fetch the bits you need.

Although I know it adds overhead to the Fresh team to maintain these, but plugins should be a very thin layer over another library anyway, and as Fresh is an opinionated framework, it seems reasonable to me to have an opinionated set of plugins included for common functionality, so you can get going as easily as possible.

mcgear commented 1 year ago

Its always felt a little funky to me that the twind plugin was a part of the Fresh codebase. I would think that the out of the box plugins would be ones that add deno functionality to fresh, or that provide core new "OptIn" features for fresh, say maybe an Authentication/Authorization core.

For this reason, it does seem like Deno KV Oauth should be an official plugin, and the twind plugin would probably live on its own.

iuioiua commented 1 year ago

Hi all, I spoke to Marvin. He's in favour of this proposal. For anyone wanting to implement this, please let us know in this issue. I'll happily take a look at the PR once submitted. It should be a relatively easy task - copy/paste.

mbhrznr commented 1 year ago

sign me up!