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

Decorators & Plugins #18

Closed
togmund opened this issue Mar 26, 2021 · 3 comments
Closed

Decorators & Plugins #18

togmund opened this issue Mar 26, 2021 · 3 comments

Comments

@togmund
Copy link

togmund commented Mar 26, 2021

Summary

I see that you have three tickets for modules you intend to provide within Worktop:

I'm a fan of the Worktop project already and am wondering if you've considered the Fastify Decorators and Plugins approach to those kinds of modules:

Rather than implementing a list of modules, create structured entry-points for extending Worktop. Then if a module like CORS is required for a particular use-case, you bring in and register or decorate that module. (Like fastify-cors)

I have a rough fork of Worktop where I've implemented a simple decorator method on the router (see the example below). If you're accepting proposals and contributions, I'm happy to clean it up into a PR for your consideration!

Great work!

Example

import { Router } from 'worktop'
import { listen } from 'worktop/cache'

const API = new Router()

// Decorate an object for this API instance
API.decorate('processObject', process)

// Decorate a function for this API instance
API.decorate('setSeveralHeaders', function (res, [header, value] = []) {
	res.setHeader('foo','bar')
	res.setHeader('biz','baz')
	res.setHeader('wing','bird')
	if (header && value) {
		res.setHeader(header, value)
	}
})

API.add('GET', '/greet/:name', (req, res) => {
  // Retrieve the decorated object
  const { title } = API.processObject
  // Invoke the decorated function
  API.setSeveralHeaders(res, ['process-type', title])

  res.end(`Hello, ${req.params.name}!`)
})

listen(API.run)
@lukeed
Copy link
Owner

lukeed commented Mar 26, 2021

Hi, thanks!

I have considered this -- mostly in terms of routing. I've had this internal battle about it for Polka, too.

The problem is that decorator support is spotty & spooky to a large number of people still. It also raises a bunch of TS typing implications, which I've not really looked into as this is my first framework claiming 1st class TS support.

I don't want to say much about this right now, but I will mention that I've been toying around with compile-time optimizations. So, again with routing, the regexparam code should/will not be in the generated bundle at all. Most of the router class will disappear too actually.

I'll leave this issue open to collect feedback from others, but my personal take on decorators has been a mixture of 🤩 + 😬 for years now -- haven't seen much happen to alleviate the concerns.

@lukeed
Copy link
Owner

lukeed commented Mar 26, 2021

To be clear, I'm not interested in the proposed example snippet (or plugins), as that can be done w/o a decorator:

const API = new Router;

API.setSeveralHeaders = function (res, [header, value] = []) {
	res.setHeader('foo','bar')
	res.setHeader('biz','baz')
	res.setHeader('wing','bird')
	if (header && value) {
		res.setHeader(header, value)
	}
};

I also think this particular snippet could just be replaced with the Router.prepare hook, if need be.

In any event, a core motive for worktop is to do as little runtime work as possible. That should be every framework's goal -- but it's especially true here given that there's a CPU limit. This is why plugins (in the Fastify sense) aren't going to be considered either... it's runtime code that builds up a state only to be thrown away immediately after. All it really does is eat away at that CPU budget. And finally, on this point, it's why worktop offers tree-shakable submodules. Functions are cheap & easily composed. Because you have to involve a bundle-step with worktop (intentional, no way around this), all your methods/utilities should be set up this way.

The decorator API I was considering was something like this:

@GET('/hello/:name')
const list: Handler = async (req, res) => {
  // I can skip assigning this to Router
  res.end(`hello ${req.params.name}`);
}

So, I think I'll actually close this issue to avoid the confusion. I'll open another to gather feedback about ^this line of thinking for decorators.

What do you think?

@lukeed
Copy link
Owner

lukeed commented Apr 2, 2021

Will close this for now. Only recently discovered that decorators are meant to be with/on classes only 💔
Relevant Proposal: https://github.com/wycats/javascript-decorators

Worst case scenario – I implement my idea via typedocs or comments... but maybe there's something else can wiggle together.

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

No branches or pull requests

2 participants