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

Documentation for Feature class #565

Open
aneilbaboo opened this issue Dec 11, 2017 · 36 comments · May be fixed by #1231 or #1232
Open

Documentation for Feature class #565

aneilbaboo opened this issue Dec 11, 2017 · 36 comments · May be fixed by #1231 or #1232

Comments

@aneilbaboo
Copy link

aneilbaboo commented Dec 11, 2017

The Feature class seems like a key conceptual and organizational unit. It's used throughout the code base, but there's no documentation. Adding documentation for this particular item would be pretty high impact, IMO. It would make it much easier to use for beginners.

Here are some thoughts for an outline:

Feature Class Documentation

  • Overview
    • Purpose
    • Client Feature vs Server Feature classes
  • Server Feature
    • Arguments
      • schema
      • createResolversFunc
        • When is it invoked?
          • What should it return?
          • Using pubsub
            • explain the pubsub argument
        • createContextFunc
          • when is it invoked?
          • what should it return?
        • middleware
        • Hierarchical composition
          • explain new Feature(counter, post, user, graphqlTypes); in modules/index.js`
  • Client Feature
    • Arguments
      • navItem
      • tabItems
      • route
      • reducer
      • features
    • Hierarchical composition
  • Tutorial:
    • add a server feature
      • add a model using a migration
      • add graphql schema
      • add resolvers
    • add a client feature that uses the server feature
This was referenced Dec 11, 2017
@mitjade
Copy link
Collaborator

mitjade commented Dec 14, 2017

@aneilbaboo Thank you for the outline. I agree that this kind of documentation is of great importance.

Currently we are busy with improving other parts of this kit, but would be really grateful for any kind of help in this area. If you would be willing to start writing docs for some of this parts on your own, we can help out and fill in the missing parts, when time permits us.

@mitjade
Copy link
Collaborator

mitjade commented Dec 14, 2017

For starters we can create a doc directory and write .md files there and when something is done, we can link it to the README.md. Later we plan to have a separate site for docs.

@aneilbaboo
Copy link
Author

Hi @mitjade I haven't quite grokked what is going on here myself. I'm a React newbie, so if there's someone else reading this 👋 maybe they can take a crack?

I'm evaluating a number of different frameworks right now. If I settle in on this repo, I might be able to contribute.

@aneilbaboo
Copy link
Author

For example, I only have a vague understanding of what the feature class is doing, where it is invoked, when to use it, etc. If someone from the team could fill in at least the overview section, that would be a good start.

@mitjade
Copy link
Collaborator

mitjade commented Dec 16, 2017

@aneilbaboo Thank you for your feedback. Will try and put something together when time permits us.

@verdverm
Copy link
Contributor

I can put something in about Features with my upcoming auth docs. Here are some pointers in the meantime:

  • Features are different in the client and server. You could theoretically have multiple types of features on either end. They can be fractal, that is, a feature can have sub features.
  • Features are basically a predefined set of options that get recursively merged. Different params on the frontend and backend. You can change theses.
  • Feature code is found at the root of the client/server module directories
    • Definitions are in connector.js in the server, connector.web.jsx in the client
    • Feature inclusion is in the index.js
    • Actual features import the connector.js file and add the params they want
  • Common code uses Feature params to do things such as, create the context for the resolvers or build menus in the client.

In the auth-upgrades branch (https://github.com/sysgears/apollo-universal-starter-kit/tree/auth-upgrades)...

I have some examples of:

  • conditional features based on settings (see the {client,server}/modules/index.js files)
  • fractal features (see the {client,server}/modules/entities dirs)

@aneilbaboo
Copy link
Author

aneilbaboo commented Dec 19, 2017

FWIW, I've settled on this kit. I have to say, now that I've wrapped my head around React and looked at a number of other starter kits, I really love the approach taken here. I'm going to write up some documentation.

One thing that is a bit confusing for newbies is the naming convention. Folders in the modules directory export Feature instances and the Feature class is found in connector.js. It causes a bit of unnecessary boggle having all these names floating around.

@verdverm @mitjade - what do you think of unifying these names under "module"? Feature=>Module, connector.js=>module.js?

Or to make this crystal clear, Feature=>ServerModule+servermodule.js / ClientModule+clientmodule.js?

Changing the names will make the docs more readable too.

@larixer
Copy link
Member

larixer commented Dec 19, 2017

@aneilbaboo The thing is this schema is platform agnostic. It is fully abstract concept. I call it feature-modules. But I agree that naming can be improved. So no reason to have servermodule.js or clientmodule.js, this unnecessarily shrinks down the meaning, because feature-modules can be used in other places of the kit and one feature-module can be a host for many other sub-feature-modules.

@larixer
Copy link
Member

larixer commented Dec 19, 2017

Feature-modules is an attempt to have Interface-like concept in JavaScript. We connect all the modules in client/modules to the other parts of kits code using one Interface declared in connector.js. Each such module must implement this Interface. When we have Interface we know how to write new module-feature implementing that Interface and how to use all feature-modules that implement that Interface in the rest of the kit

@larixer
Copy link
Member

larixer commented Dec 19, 2017

@aneilbaboo All ideas how to improve the naming as well as writing docs are highly appreciated. It is difficult though, considered this kit is used also with TypeScript and with Flow that really have interfaces, but we needed something more portable and available in vanilla JavaScript

@aneilbaboo
Copy link
Author

Feature-modules is an attempt to have Interface-like concept in JavaScript.

Yes, and they do actual work. What I really like about it is how they formalize the structure of the project, and enable composition. That's more powerful than just being interfaces. It's a key selling point of this kit, IMO, and the fact that you offer the CLI for generating and wiring them in to the infrastructure is great, especially for newbies.

Honestly, I think think this is the best kit out there. The weakness is documentation and naming. I could see how the CLI would expand over time to include more and more generator patterns, and maybe plugins. React apps ought to be as easy to spin up as Rails. It feels like your module concept is a step towards that.

because feature-modules can be used in other places of the kit and one feature-module can be a host for many other sub-feature-modules.

I see. I agree with the thrust: it should be clear that Modules are an organizing principle. (By the way, I prefer "modules" to "features" because it conveys the idea of "self-contained package"; "feature" does not). I think the way to do this is to tell that story right at the beginning of the documentation, and make that the point of this kit.

It's not great to different classes with slightly different functionality in the same project. But, if the naming is fixed (Feature=>Module; connector.js => module.js), this isn't terrible. The folder and file names will tell the story.

Anyway, I'm willing to take a crack at this, and write up some documentation if you're game.

@mitjade
Copy link
Collaborator

mitjade commented Dec 19, 2017

@aneilbaboo That is great news! If you are interested in things to come for CLI generation, check out #524 for CLI that generates full CRUD. Far from complete, but I already use it in one of my projects.

@aneilbaboo
Copy link
Author

Cool. This module concept is really powerful. Imagine an NPM-like repo. I think there's a business here. I have a bunch of ideas, if you want to jump on a chat sometime.

@mitjade
Copy link
Collaborator

mitjade commented Dec 19, 2017

This kit is full of hidden undocumented gems 😉. @Vlasenko is the mastermind behind it all.
I too am full of ideas, but our current plan is to finish what was already started, so we do not have too many unfinished features.

@larixer
Copy link
Member

larixer commented Dec 19, 2017

@aneilbaboo We have a chat for this starter kit here:
https://gitter.im/sysgears/apollo-fullstack-starter-kit
We carry out general discussions and address quick questions there.
Yeah, NPM-like usage of these modules was always in my mind and still there, but implementing this will need considerable efforts.

There are too many things that need to be handled, so we try to help on all the directions. At the same time we understand that there are many things to be finished and there are still many challenges that need to be handled to make this starter kit more useful.

@larixer
Copy link
Member

larixer commented Dec 20, 2017

@aneilbaboo I think naming is very important, so we should probably think and agree on some good name for this concept of organizing code and what filenames should be used in our implementation. Both the name of concept and file naming should reflect the idea well and should be easy to remember and distinguish from other concepts.

@larixer
Copy link
Member

larixer commented Dec 20, 2017

I'm thinking about renaming it as follows:
src/client/modules/connector.js -> src/client/plugins/plugin.js
Though, the name Plugin looks very ordinary at least the meaning will be obvious, I guess.

@larixer
Copy link
Member

larixer commented Dec 20, 2017

I'm fine with src/client/modules/connector.js -> src/client/modules/module.js too

@aneilbaboo
Copy link
Author

aneilbaboo commented Dec 20, 2017

@Vlasenko, good thought. I like Plugin too (better than "interface" since interfaces have no implementation). Module conflicts with the Node.js concept of modules.

So then we would have these changes:

  • file system:
src/client/modules/ => src/client/plugins/
src/server/modules/ => src/server/plugins/
src/client/modules/connector.js => src/client/plugins/plugin.js
server/client/modules/connector.js => src/client/plugins/plugin.js
  • class names

    • Change class name from Feature=>Plugin for server and client classes
  • variables

    • Change 'modules' and 'features' to plugins
    • Change 'module' and 'feature' to plugin

Are you comfortable with this? I could see this being a bit disruptive to existing work in progress.

@larixer
Copy link
Member

larixer commented Dec 20, 2017

@aneilbaboo Yes, I'm comfortable with this and yes it is disruptive to current WIPs. And I would love to hear opinions. @mairh @mitjade @verdverm @johnthepink do you have some opinion on this matter?

@johnthepink
Copy link
Collaborator

I don't mind the current naming convention, but "Plugin" does feel like it may better communicate to newcomers what we mean when we say "Modular".

@mitjade
Copy link
Collaborator

mitjade commented Dec 21, 2017

I agree with @johnthepink.

@mairh
Copy link
Member

mairh commented Dec 21, 2017

Fine with me.

@aneilbaboo
Copy link
Author

Great. I'll submit a PR with the refactor and another PR on top of that with the documentation.

@verdverm
Copy link
Contributor

verdverm commented Dec 23, 2017

I'm not sure about the naming change yet... it's one of the (2 or 3) hardest problems in CS ;]
This is probably an important decision we shouldn't rush. it will cause havoc on a bunch of WIPs, so we do need to find a good time to do the change. We should also be mindful in creating new WIPs as the time approaches.

This is how I have been thinking about it:

modules are one of the major concepts, they seem like things you can turn on and off.
I think the word plugin better describes implementations of modules or enhancements to modules. For example, when we abstract the storage engine from being just SQL/knex to enable multiple, concurrent usage of databases and other storage types (NoSQL, S3, ES)... I think of storage as a "module" and the different implementations as a plugin.

feature doesn't quite describe what it does. You can have different and multiple kinds of features. connect.js describes how features are combined, but isn't a great name for the file.
I support changing this to something.

Some examples I have been thinking about as I think about this naming change.

  1. Users, Groups, Organizations, Bots, and other entities

This is like a module with features you can turn on and off. Maybe you want users and groups, but no orgs. You could have different plugins for auth methods and storage of secrets (Vault anyone?.

  1. Storage engines

GraphQL is cool because you can use anything that returns data in your resolvers. This could be SQL, NoSQL, Cloud storage solutions, Blockchains, any other API, and just about any function that returns some data. So we want to abstract this and have something that allows for a plugin style for these drivers. This would mostly be about config and initialization.

  1. What seem most like modules, as it relates to the codebase today
  • mailer
  • counter
  • entities (users, groups, ...)
  • authentication
    • lots of features and plugins
  • authorization
  • subscriptions
    • plugins here might be (stripe, paypal, a pay-as-you-go solution, crypto?!)
  • resources
    • posts
    • uploads
    • anything owned by an entity, this is application particular, but there are some common ones
  1. I really like Kubernetes as a model.

They have an excellent third-party-resource system and plugins. There are a lot of people who have put a lot of thought into naming things. We should see how they came to their deciscions. Overall, Kubernetes is a significant project and run quite well. I think the community and how the project has evolved is a goto example for open-source these days. I'm all about having weekly community meetings and dividing into SIGs if/when this project gets real popular with lots of contributors. We should think about having a develop branch too, and using master for releases. OK... went of on some tangents there...


So yea, what do we want to call things?

Good question

@verdverm
Copy link
Contributor

Hmmm, maybe we pull the connector.js files into the common directory.

Something like

src/common/
  features/  (or some other name t.b.d.)
    server/
       default.js   (current `client/modules/connector.js`)
       some-other-server-module-type.js
       ....
    client/
       default.js  (current `client/modules/connector.js`)
       some-other-client-module-type.js
       ....

This way

  • we retain the concept
  • collect them into one place

Now, where and how are the features used in the rest of the codebase?

That's a question I have yet to understand the answer to.

How would different server modules (substitute client as well),
which use different "features" be combined, interact, and be required / dependent ?

@mitjade
Copy link
Collaborator

mitjade commented Dec 24, 2017

@verdverm I've been thinking of this renaming as well and you raised a few valid points. Maybe we need to step back a bit before we do mayor renaming changes and first see where we want to go with this in the long run.

@larixer
Copy link
Member

larixer commented Dec 24, 2017

@verdverm I don't think we should separate out connectors from features that use them. Connector is an interface how the feature can be connected to outside world. We have two set of features now - server features and client features, but we plan to have more. For example user feature might have many sub-features and they are going to have the following structure:

src/client/modules/
   user/
      modules/
        google/
        facebook/
        jwt/
        session/
      connector.js
      index.js
   connector.js
   index.js

@larixer
Copy link
Member

larixer commented Dec 24, 2017

@aneilbaboo @mitjade I'm thinking about this renaming too. And I think we don't win much by renaming everything into plugin. Rather I think I should take time to carefully document current approach first and then we will think about renaming again. I will try to do so on the second week of January, as I'm on a vacation right now

@aneilbaboo
Copy link
Author

@Vlasenko @mitjade @verdverm

Good thoughts. I think it's a good idea for existing team to drive on a change as sensitive as this. I'm going to step back

Let me just give my newbie perspective, as you think about it. I'm just one data point, so please take this perspective with a grain of salt:

I was really confused initially because of the naming scheme. I scan code and documentation to get a high level view of a project. Scanability is useful for new users, but also for reducing cognitive load for existing ones.

Currently, there is a class (Feature), exported from a file with a different name (connector.js). Instances of Feature are exported from inside the modules folder. So the same concept has many different names depending on whether you're referring to the class, the file or instances of the class. In addition, module sometimes refers to a node module. One has to read carefully to understand context. This is very confusing.

Merely documenting this isn't a great solution, IMO. A new user scanning (i.e., not reading carefully, which is most people) the documentation will think there are more concepts than actually exist, and might struggle to understand the relationship between them: "How is a Feature different from a module?", "Is there a difference between 'feature modules' and 'modules'?", "Why is this called connector?", "Is there a relationship between the sql/connector and the client/src/connector?", "why do the modules export Feature instances?", etc.

Then, either the documentation reminds the user how the words are related throughout, or one section explains that Feature=connector=modules. So the choice is between bloated documentation or risking a confused reader. Alternatively, if someone carefully reads the documentation, they'll wonder why so many different names are used for the same thing. Then, even if they read the section explaining these things are the same, they may forget.

Also, using multiple names for the same concept reduces the number of names available for new concepts. What if an actual "connector" is required at some point? It's possible to envision a future need for cross-cutting "features" which work across modules, for example. Better to straighten out names sooner rather than later.

If the goal is to provide continuity for existing developers, it's probably better to keep the naming scheme as is. If it should be easy for new users to understand and start using the kit, renaming is going to help a lot, IMO.


Anyway, guys, please take that only as one "product feedback" data point. @Vlasenko, hope you have a good vacation!

@larixer
Copy link
Member

larixer commented Dec 25, 2017

Thank you @aneilbaboo. Yes, it is understood the naming is not perfect and confusing now. We are going to fix this soon.

@verdverm
Copy link
Contributor

I was thinking about how to make the connectors more duck-type-y.

I've experimented with merging different types or exporting multiple merged Features from the same module.

What I'm thinking something like modules exporting the config object, rather than wrapping it in the "Feature" at the start. Then when / where a feature is needed and paired with a module, the correct kind can be used. And because they are exported under default names, you can call "Feature" whatever you want when you import and use them.

This allows the module to be configure for multiple consumers or features. I'm still confused by how flexible it is. A module and be both a "module" and a "feature(s)" have sub-features/modules and import other module pieces or user their "Feature" to render something (like a menu)

If we move to objects being exported rather than features, I think the merging and duck-typing will be easier. Maybe the connector.js can merge fields, so that it doesn't need a predefined set? Maybe we use something like prop-types to ensure a feature has the necessary config provided by a module when it's instantiated?

It's pretty deep, and way awesome! I think some docs and examples would go a long way for sure!

@leviwheatcroft
Copy link

has anyone started on this? Just wondered if there's a fork / branch / PR lying around somewhere with an unfinished attempt.

Failing that, where would be the best place to see an implementation of a Feature subclassing other features ?

@larixer
Copy link
Member

larixer commented Feb 13, 2018

@leviwheatcroft We use feature user subclassing other features (jwt, sessions, facebook, google):
#644

@leviwheatcroft
Copy link

Just trying to get my head around features like @aneilbaboo, so there's still big gaps in my understanding. This might be vaguely related to what @verdverm suggested re: exporting objects.

It looks as though some of the confusion might stem from the fact that the main purpose of the Feature class exported by connector.js in both the client and the server is to connect modules together. Other than providing getters the class doesn't do anything else. So whilst new Feature(declaration) does return a feature, it implies that your declaration will be decorated with an API, which is only somewhat accurate.

If the Feature class were to provide any logic, perhaps that would be better subclassed, so class MyFeature extends Feature { ... }. If it's not providing anything, then modules exporting a plain object might be best.

Then connector.js should return a connect function with the same pattern as the current Feature constructor.

So if you were defining features with plain objects you'd have:

import connect from '../connector'
const MyFeature = { ... }
export default connect(MyFeature, MyOtherFeature)

You could still have a Feature class in connector.js if that's the most appropriate, but beginners / visitors wouldn't really have to think about it. Instead of worrying about what Features, connectors, and modules are, you'd only be thinking about modules, and connecting them together.

@verdverm
Copy link
Contributor

I've generally been thinking Connectors are objects with a "Connect" function and some collection of arbitrary objects, while Features declares/enforces some interface or "API" for the connected (formerly arbitrary) objects. This makes sure the object has some set of attributes at insertion time. Separately, and after the fact, you should also be able to "Get" all arbitrary objects which have some set of attributes, i.e. duck-typing.

Connectors or Features can show up anywhere in the directory / conceptual structure. They get brought together as the code / files are imported down to the "root" (This is much like index.js is used generally in JS). At the "root", all of the client/server modules should be present. Today, consumers import this file directly (which is causing some circular imports). With connector-js, the "Connect" function is available (and used in the client/server feature links below). This does however, require some effort on a module's developer to gather and save these components (see last link). There is also some complication around when/where to call the Connect function on a root Connector (and the recursion, which is not happening because I ran into some issues ((which I think I just resolved in my head ;)))

Here is an example of how I've started to organize the connectors after refactoring to have modules with client/server code co-located. In also uses connector-js with the server/client Feature classes extending the Connector class. The following links are some junction points or key files which should help you navigate the connector usage.

Root Connectors (the .js files in the modules dir)

ClientFeature ServerFeature

Two connected client components

This was referenced Feb 12, 2023
Closed
@pooriyasafaei pooriyasafaei linked a pull request Feb 12, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants