fastify / fastify-plugin

Plugin helper for Fastify
MIT License
196 stars 42 forks source link

Rename this module #207

Open mcollina opened 1 year ago

mcollina commented 1 year ago

From a discussing with @lirantal on twitter: https://twitter.com/matteocollina/status/1610577242844708865?s=46&t=wpaDKCujckm07OZ7JAa3hg:

Unfortunately there is. I made some terminology mistakes at the beginning and they stuck, eg “everything is a plugin”. But also “plugins are encapsulated by default”. “fastify-plugin” is for creating reusable modules, not for application builders.

I think we should rename this module to @fastify/meta.

jsumners commented 1 year ago

I think this would cause way more confusion that it solves. We have already established that fastify-plugin is used for module creation. It's a core concept in the framework and ecosystem (also why we didn't namespace it with the other modules). Changing the name would be a bit hit in education (which we already have a problem with) and plugin maintenance. Just look at our org and all of the modules we'd have to go update; the project is equal to or greater in scope as the namespacing project.

If it were to be renamed, though, I'm not sure about @fastify/meta. How does that fix the "what does this tool do by only reading its name" problem? It adds metadata, sure, but it also changes the way a plugin operates (encapsulation breaking). I don't currently have a better name, but I don't think "meta" is it.

delvedor commented 1 year ago

I agree with what @jsumners said. Yes, in retrospect, it's a bad name, but changing it now could cause more problems than what it would actually solve. If we decide to rename it, it should be a name that is so good that it's impossible to say no :P

Some ideas:

fox1t commented 1 year ago

I agree with @jsumners and @delvedor, but, as I said on that Twitter thread, it is impossible for me to unsee the issue with that name. I want to give it a shot: we can use the "opposite" of encapsulate.

Self-explanatory enough.

Eomm commented 1 year ago

I would add to the table:


I think after collecting some ideas, we could just vote the proposed option 🔢 / or to not rename it 👎🏽

climba03003 commented 1 year ago

Beware of we provide a option for encapsulate, so it by default exclude from encapsulation but also allowed to create encapsulation.

Uzlopak commented 1 year ago

I had a discussion a week ago wth liran tal about that issue in linkedin. https://www.linkedin.com/posts/talliran_hey-fastifyjs-devs-a-thought-would-it-activity-7014228764804952064-bcKB?utm_source=share&utm_medium=member_desktop

I think the name, fastify-plugin, is good.

delvedor commented 1 year ago

Beware of we provide a option for encapsulate, so it by default exclude from encapsulation but also allowed to create encapsulation.

I would avoid this, encapsulation by default is a design choice.

climba03003 commented 1 year ago

I would avoid this, encapsulation by default is a design choice.

Maybe you somehow mistaken what I am saying? It is a option in this package. https://github.com/fastify/fastify-plugin#encapsulate

So, any name that imply it must escape the encapsulation will be wrong because you have the ability to control the behavior.

jsumners commented 1 year ago

Here's the thing, "plugin" has two simultaneous definitions, with one being a subset of the other:

  1. a simple function that encapsulates logic (e.g. other plugins and routes)
  2. a simple function that encapsulates logic in a distributable package with corresponding metadata (i.e. fastify-plugin has been applied to it)

The fact that a plugin can have its context exposed to ancestor contexts or not is not really the point.

mcollina commented 1 year ago

@jsumners +1. That's pretty much the sum of the discussion. This is mostly a utility module so that all the metadata needed by Fastify can be set in a convenient way instead of using global symbols.

(This is not part of Fastify to avoid individual packages to depend on Fastify itself.)

The question is how we fix this. It's confusing for most people, and I think we should tame this problem. We do have a slot for releasing a Fastify v5 release next year (to drop both v14 and v16), so we might think of a breaking change as well.

jsumners commented 1 year ago

@fastify/annotate-plugin?

mcollina commented 1 year ago

@fastify/annotate?

jsumners commented 1 year ago

@fastify/annotate?

But what does it annotate?

mcollina commented 1 year ago

Plugins!

Uzlopak commented 1 year ago

Tomorrow we will not only annotate some symbols but encapsulate some additional checks and then we need a new npm package covering that behaviour.

jsumners commented 1 year ago

Plugins!

I know, but the name does not make that clear. When people are looking through our list of modules, the name of those modules should give some indication of their purpose.

mcollina commented 1 year ago

How about @fastify/add-metadata?

jsumners commented 1 year ago

I still think it isn't completely clear that it is meant to add metadata to plugins, but if everyone else likes the name then I am not a blocker.

fox1t commented 1 year ago

What about @fastify/plugin-metadata? So we can conserve the plugin part in the name (which I agree with @jsumners is important) and we say what the package does (manipulates the plugin metadata).

lirantal commented 1 year ago

That statement about hard things in computer science is... not wrong 😅

To add a bit more context from the perspective of someone coming anew to Fastify and its conventions, it root confusion is that @fastify/plugin is not meant to be used by app developers, but rather by plugin developers. I guess the exception is when you define a plugin in your app.

Given the above statement, not sure @fastify/plugin-metadata makes that clearer. Some options that might be better:

I'm in favor of the plugin-export option due to familiar naming and how the naming keeps clear of the intent without leaking internals of what is happening by it such as annotations, metadata changes, etc.

Happy weekend friends ❤️

Uzlopak commented 1 year ago

fastify-module.

d1b1 commented 1 year ago

As a newbie to fastify and its plugin system, the naming conventions are lead into how to think about packages. Given that plugins carry its own baggage, a clear README that makes sure developers can read and understand the fastify approach might be the best solution.

'@fastify/plugin-expose' is the closest, and would make me dig into understand the plugin framework in fastify

Maybe '@fastify/plugin-extras' ?

lirantal commented 1 year ago

Is @fastify/plugin-export not a better alternative @d1b1 ? As a module author, I don't see anything about the naming plugin-export that would peak my curiosity to further investigate what it does under the hood. It's superior in that regards to expose which leaks a potential implementation detail where-as export is quite neutral.

d1b1 commented 1 year ago

Gotcha, in the end the name is short enough, that anyone who cares has to go into the Readme. So yes, 'export' does the same thing, and better. Export is a node term as well, so leads the dev to thinks node vs export data.

lirantal commented 1 year ago

export is also an ESM term so it works for everyone in JavaScript :-)

mcollina commented 1 year ago

I think @fastify/export is nice and short. Wdyt folks?

58bits commented 1 year ago

@fastify/plugin-helper? [posts suggestion and ducks]

Whatever the name, I agree with @fox1t It would be helpful to preserve the 'plugin'- portion of whatever the name becomes.

I guess the difficulty (as @jsumners highlights) comes from the fact that the plugin is doing two things - determining scope via yourPlugin[Symbol.for('skip-override')] = true and providing meta information - name, dependencies, Fastify version etc.

jsumners commented 1 year ago

Someone (@mcollina) should put out a poll with the options:

Let the wider community decide (since they are very likely not reading this thread).

After that, the question becomes who will take on the work.

mcollina commented 1 year ago

What should I use to create a poll? Happy to promote it on my social media.

jsumners commented 1 year ago

What should I use to create a poll?

Not sure. Does the OpenJSF have a tool we can use?

Happy to promote it on my social media.

Exactly. You have the most reach for such a poll.

mcollina commented 1 year ago

I'll get it done

Joabesv commented 1 year ago

Hi guys, just wanted to know if the module rename will happen? or it got put aside

mcollina commented 1 year ago

It will happen. I've been sidetracked by various things. At this rate, it might happen when we ship Fastify v5.

gurgunday commented 4 months ago

I vote for @fastify/export 😁