search

loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.96k stars 1.07k forks source link

RFC: Reorganize code & docs along abstraction levels #5550

Closed bajtos closed 4 years ago

bajtos commented 4 years ago

In my view, loopback-next is currently a mixed bag of low-level building blocks like @loopback/metadata and high-level framework packages like @loopback/core and @loopback/rest. Our documentation is often describing concepts from different abstraction layers in the same place, which makes it IMO more difficult for newcomers to learn how to use the framework.

I am proposing to reorganize our monorepo structure and the documentation in such way, that the documentation for framework consumers describes primarily framework-level APIs and deemphasizes the lower-level building blocks.

For example, we should stop referring to @loopback/context in our documentation and tell users to import DI/IoC features directly from @loopback/core. This way new-to-intermediate users don't need to understand the complexity of how LoopBack is composed internally. Of course, we should provide enough information for expert users to allow them to understand our building blocks and how they are composed together, but that should be in the part of the documentation that's clearly aiming at very advanced developers.

I am proposing the following rule of thumb: if a higher-level package is re-exporting all public APIs from another package (e.g. @loopback/core re-exports @loopback/context) or is encapsulating another package as a building block (e.g. @loopback/rest uses @loopback/express), then the later package should be moved to "building blocks" and our documentation should be updated to reference the former "framework" package in most places.

Proposed taxonomy of monorepo packages (not a complete list):

Framework

I am proposing to move these packages from packages/{name} to framework/{name}.

Building blocks

I am proposing to move these packages from packages/{name} to foundation/{name}. (I am not entirely happy with the name foundation, feel free to propose a better one.)

The documentation for this packages should primarily aim at people using these packages outside of LoopBack context, see the concept of microsites I mentioned in https://github.com/strongloop/loopback-next/issues/5113.

Utilities

I see these packages as mostly standalone additions to the framework that are not mandatory to use. Documentation-wise, they can be documented independently from the "main" framework documentation, but the docs should show how to use these utilities in the context of a LoopBack project. As a nice-to-have feature, the docs can also show how to use these utilities in non-LoopBack projects too.

Let's move them from packages/{name} to util/{name}.

Extensions

All existing packages in extensions/* belong to this category, we may want to move some additional packages from packages/* into this category.

Documentation wise, I think these extensions should be integrated into the rest of the docs, most importantly into "How to guides" (see also https://github.com/strongloop/loopback-next/issues/5549).

Next steps

raymondfeng commented 4 years ago

I agree with the overall direction to reorganize the packages into a few more locations. The packages/ is becoming a kitchen sink.

I have some slightly different opinions on how they are grouped. For example,

bajtos commented 4 years ago

I like the idea of introducing more groups. I think there are two ways how to look at this:

Layers of abstractions

What parts are internal building blocks and what parts are the framework people should import API from?

I think it's easy to agree that @loopback/metadata belongs to foundation that most LoopBack users should not need to know about, while @loopback/rest is a part of the framework that application developer need to understand.

Functional areas (groups)

From this angle, we want to group packages by the functional areas they are dealing with. Some packages are related to component wiring (context, IoC, etc.), some are for REST-related things (rest, rest-explorer, http-server), and so on. Different areas typically have peer relations among themselves, e.g. @loopback/repository is often used together with @loopback/rest, but they don't depend on each other.

Most areas are touching both the platform and the framework. For example, the REST API layer has @loopback/rest at the framework level and @loopback/http-server at the platform level.

Next steps

In this issue (epic), I'd like to focus on the distinction between the framework and the platform and how to apply this distinction to our user-facing artifacts (documentation, tutorials, example applications).

I see this as the first step on a longer journey. As part of this story, we identify the platform packages and make the necessary updates. Later (in follow-up stories/epics), we can identify other areas and carve them out into well-designed and well-documented groups.

For the first round, I am proposing to mark the following packages as part of the platform and update our docs & examples to stop referencing them directly:

A discussion point: Do we want to put platform packages into a different place than framework packages? I'd like to, because I think it would be a good way how to remind us which packages are considered to be a part of the platform vs. the framework. However, the list above contains packages from different groups (functional areas). Should we create two places (platform, framework) for each goup?

Example 1:

platform/
  inbound/
    express/
    http-server/
  wiring/
    context/
    metadata/
framework/
  inbound/
    rest/
  wiring/
    core/

Example 2:

inbound/
  platform/
    express/
    http-server/
  framework/
    rest/
wiring/
  platform/
    context/
    metadata/
  framework/
    core/

To be honest, both examples feel over-engineered to me. Can we mix all platform-level packages into a single place?

platform/
  context/
  express/
  http-server/
  metadata/
framework/
  inbound/
    rest/
  wiring/
    core/

Maybe we should defer the task of moving files around until we have a better picture of all functional areas (groups) we have in our monorepo?

mitsos1os commented 4 years ago

kudos to @bajtos and @raymondfeng for proposing the new documentation structure. Building a strong community is crucial to the framwork's acceptance and reputation and having a well structured documentation that welcomes new users is one of the most important parts for this. In my opinion both approaches could be a match as described very nicely here: #5549

I agree from what I have read that Loopback's main role is an infrastructure framework that could be the glue for everything someone would like to build. However it is also true that the extensive majority of the documentation and framework description is targeted at REST server implementation as correctly described in #5551

It is really great to support all the glue infrastructure but I believe it is harder to stand out from the rest of the crowd (which by the way is huuuuuge in NodeJS!!! 😄 ). The truth is that the majority of real word web apps is actually running in REST, so targeting and being the "framework to go" for REST implementation for NodeJS is actually a pretty solid goal. Think of Python users. When they need something robust for a REST server implementation with all inclusive production ready functionality Django is the way to go!

I would like to also propose / discuss some thoughts I have and also mentioned in the maintainers' call that would overall help in the progress of LB4.

Regarding the documentation :

Apart from the documentation, other things can help promote the framework:

Finally regarding framework functionality I find the following pain points coming a heavy user of LB3:

Forgive me for the long text! I hope I have made my views clear on the whole LB4 matter and I am looking forward to discuss any opinions you might have on the discussed matters!

Thanks!

raymondfeng commented 4 years ago

To be honest, both examples feel over-engineered to me. Can we mix all platform-level packages into a single place?

platform/
context/
express/
http-server/
metadata/
framework/
inbound/
rest/
wiring/
core/

I would prefer a flat structure:

platform/ 
  metadata/
  context/
  core/ (I feel core should be in the platform)
  boot/ (See https://github.com/strongloop/loopback-next/pull/5618)
  // The next two are interesting. They seem to be transport related
  http-server/
  express/
framework/
  rest/
  repository/
  service-proxy/
  authentication/
  authorization/
  security/
  ...

BTW, I like the diagrams. We should continue to expand from https://loopback.io/pages/en/lb4/imgs/lb4-high-level.png.

raymondfeng commented 4 years ago

image

raymondfeng commented 4 years ago

Once we settle on what's in the platform, we probably should have a @loopback/platform package that exposes public APIs from the platform modules and have all higher level packages to use @loopback/platform as dependencies.

raymondfeng commented 4 years ago

See https://github.com/strongloop/loopback-next/pull/5625

bajtos commented 4 years ago

Yeah, we need to build a shared understanding, the term "platform" is probably not a good one. As I understand it, you are treating "platform" as one of the vertical groups, while I treat it as something that's below the surface.

Here is the distinction I'd like to capture in our directory structure:

  1. Packages that LB applications import from (@loopback/core, @loopback/rest, @loopback/repository and so on)
  2. Packages that are not consumed directly, only as a dependency of a public package (e.g. @loopback/context, loopback-datasource-juggler, and so on).

Another way how to look at these two groups:

  1. Packages that are useful only within the framework and that cannot be (easily) used in non-LoopBack projects.
  2. Building blocks that can be used outside of LoopBack-powered applications too.

As I see it, @loopback/core belongs to the first group, because it provides the structure for LoopBack applications which IMO does not make sense in non-LB projects. For example, if you are building with Express or Fastify, then you have the application object provided by Express/Fastify, you are not going to use Application from @loopback/core.

ADDED:

Also, our main framework documentation should mention almost always packages from the first (framework) group. Packages from the second group (the building blocks) should have their own documentation microsite to show how to use this blocks outside of LoopBack.

raymondfeng commented 4 years ago

Another way how to look at these two groups:

  • Packages that are useful only within the framework and that cannot be (easily) used in non-LoopBack projects.
  • Building blocks that can be used outside of LoopBack-powered applications too.

It will be a good first cut.

As I see it, @loopback/core belongs to the first group, because it provides the structure for LoopBack applications which IMO does not make sense in non-LB projects. For example, if you are building with Express or Fastify, then you have the application object provided by Express/Fastify, you are not going to use Application from @loopback/core.

I have a different view here. The core package provides common wiring patterns for applications beyond LoopBack, such as:

Even for Fastify/Express applications, the core package can be used to provide a great composition story that allows the application to be composed from multiple packages.

The Fastify/Express application concept can have a tandem LoopBack application as the IoC container.

If we think there are LoopBack specific concepts in core, we should refactor them out. One candidate can be controller.

bajtos commented 4 years ago

Looks like this topic needs more discussion to reach consensus. To be honest, I don't have enough appetite for that now.

Reflecting back on my motivations for opening this issue, my main desire is to simplify our documentation and make it easier for new users to learn about LoopBack, by reducing the amount of packages they need to know about. As I wrote at the top:

In my view, loopback-next is currently a mixed bag of low-level building blocks like @loopback/metadata and high-level framework packages like @loopback/core and @loopback/rest. Our documentation is often describing concepts from different abstraction layers in the same place, which makes it IMO more difficult for newcomers to learn how to use the framework.

I am proposing to reorganize our monorepo structure and the documentation in such way, that the documentation for framework consumers describes primarily framework-level APIs and deemphasizes the lower-level building blocks.

For example, we should stop referring to @loopback/context in our documentation and tell users to import DI/IoC features directly from @loopback/core. This way new-to-intermediate users don't need to understand the complexity of how LoopBack is composed internally. Of course, we should provide enough information for expert users to allow them to understand our building blocks and how they are composed together, but that should be in the part of the documentation that's clearly aiming at very advanced developers.

I am proposing the following rule of thumb: if a higher-level package is re-exporting all public APIs from another package (e.g. @loopback/core re-exports @loopback/context) or is encapsulating another package as a building block (e.g. @loopback/rest uses @loopback/express), then the later package should be moved to "building blocks" and our documentation should be updated to reference the former "framework" package in most places.

I'll open a pull request to capture this proposal in our docs and propose a list of packages that should be de-emphasized in our docs - it looks to me we are mostly in agreement about that. (See also #5625).

We can continue discussing other aspects of package grouping & organization in follow-up issues.

bajtos commented 4 years ago

@mitsos1os thank you for a thoughtful comment. There is a lot to unpack there, I'll try to respond with actionable proposals later this week or next week 🤞

bajtos commented 4 years ago

@mitsos1os Thank you again for the comment. Let me reply to individual suggestions you proposed.

Loobpack's documentation currently is a bit chaotic. You get lots of information spread throughout different sections of the same page, also referencing different parts of the documentation. Also it is plain text with white background for the most part. A documentation framework or custom styles should be applied to make the documentation easier to read as very well done on different frameworks ex: NestJS.

I see two topics here:

  1. How to organize the content in a better way
  2. Presentation/design/style changes

I think the first topic is best discussed in #5549 and perhaps in #5113

I am not sure if we have any existing issue for improving the visual design of our docs, could you please create a new issue linked to #5113 and ideally offer more concrete suggestions or wireframes to show the visual design you consider as a better one?

A more guided approach should be taken where it is clear for a new user what needs to be read, so that he has the required info in order to achieve a goal. Exposing him/her directly to the complete information available is overwhelming.

+1, we are discussing this in #5549

Be more careful with the actual documentation. There are quite some cases where the displayed code snippets are malformed, due to missing some - required to understand the context - code or even display wrong member names in the code example instead of the text referenced ones. This makes it even more difficult to understand because you have to check if you are missing something or it just is a wrong example

Bugs and mistakes will always slip in. Could you please open one or more GH issues to point out the specific places where the docs is missing context or showing a code snippet that does not work?

Throughout the documentation there are many cases where different approaches are proposed in order to achieve the same result. This is also confusing for new users. In constract, other framework documentation follow a more opinionated path. ex in NestJS: Transform - Validation is implemented through pipes. Authentication through guards, etc... It would be better if an opinionated way, following best practices regarding the framework architecture and scope, was suggested and be the place to look into for a user trying to find how to implement a specific scenario. If someone wants something really different, he will need to dig through the internals, so reaching this stage will probably imply that he should then be able to achieve the same result with a differnet approach than the opinionated way.

I agree with you that having too many options is not good, Decision fatigue is a real thing. Do you have any concrete ideas what steps to take to improve this aspect? If yes, then please open a new GitHub issue linked to #5113.

So what is it that makes LB4 different than the other frameworks? Having a side by side comparison with other popular frameworks, will help newcomers understand what is offered from LB4 and possibly hint them to the right direction which is not obvious from the start. Also feature comparison like all in one functionality such authentication - authorization, existence of common CRUD interface for all supported DB's out of the box, etc...

We have a comparison table for LoopBack 3 here: https://loopback.io/lb3/resources#compare We used to have a task to publish a blog post comparing LB4 to other frameworks (see #630), but we never wrote such post.

Let's open a new GH issue linked to #5113 to add a framework comparison table to loopback.io.

It is very nice to provide the possibility to integrate the ORM of your choice, but loopback-datasource-juggler is here to stay for at least some time, so formally include - propose it in the documentation and do not refer to it as legacy. This creates the feeling that there is no stable state and breaking changes will be needed in the future.

Great suggestion, here is the tracking issue: https://github.com/strongloop/loopback-next/issues/5558

Focus on best practices regarding every component of the framework. Showcasing different approaches also creates the unstable feeling... For example, having SINGLETON controllers should be the way to go and the user should be guided on how to achieve this. This is only mentioned in some small quote text somewhere in the documentation, where it states that you can do this as long as you meet the proper param injection requirements, etc....

Personally, I am not a fan of singleton controllers and param injection. Coming from OOP/SOLID background, I like to treat my controller instances as a mini-context where I can keep local state shared between private controller methods.

I had also enough exposure to functional programming style, where context is passed in function arguments. TBH, I like this style more than OOP/classes, but implementing it via static controller methods with method-level dependency injection looks like a hacky workaround to me. What I would prefer instead is first-class support for REST endpoints implemented via handler functions, preferably in a style that supports pure JavaScript (does not require TypeScript decorators). For example (cross-posting from https://github.com/strongloop/loopback-next/issues/1978, see also #2478, #2474 and linked issues):

app.route(
  verb: 'get',
  path: '/todos',
  inject: ['repositories.TodoRepository'], // a list of dependencies
  spec, // describe filter arg & return values
  handler: async function findTodos(TodoRepository, filter) {
    // ^^^ dependencies first, spec parameters second
    return TodoRepository.find(filter);
  },
});

Anyhow, this is out of scope of documentation improvements.

I agree with you that we should better describe different programming styles supported by LoopBack (controllers instantiated per request, controller singletons, etc.) and explain the pros and cons of each approach, so that users can make an informed decision about which style to use in their project.

Let's open a new GH issue linked to #5113 for the task of making singleton controllers a first-class well-documented feature.

Provide an official implementation of the popular realworld example with Loobpack 4 using best practices as showcased from the documentation. I understand that this could be considered achieved by the shopping-example, but this also goes to the beforementioned part of comparison. Being able to see exactly the same spec being implemented by different frameworks is a very good helper in demonstrating the framework capabilities and also act as a guide for people to switch over from other frameworks.

This is a great suggestion! I was not aware of realworld, I like the wide range of both front-end and back-end frameworks covered and that the backends include all sorts of technologies like Ruby on Rails, Kotlin, Rust, and many more.

I am not sure if our team will have bandwidth to contribute a LoopBack version, but feel free to open a GitHub issue for somebody from the community to pick it up.

Feature parity with LB3 should be a main target in order to create the proper circumstances for users to upgrade and switch over to LB4. For example, the Operation Hooks functionality is something quite useful in the ORM layer. An official bridge should be implemented, because just overriding all the required functions in your repository class (ex. CREATE, UPDATE, REPLACE) is like replicating the code in juggler for WRITE - before/after save hook. This was also the base for really popular implementations of soft-delete functionality etc, where the grouping of CRUD methods and their hooks is needed.

We are tracking Operation Hooks feature in https://github.com/strongloop/loopback-next/issues/1919. It has only 5 upvotes (👍) so far, which leaves impression that it's not that much popular. (E.g. when compared to ENUM feature https://github.com/strongloop/loopback-next/issues/3033 with 80+ upvotes.) I am not saying we should prioritize only by upvotes, but we do take that number into account.

Let's move the discussion about Operation Hooks to #1919.

We also have an Epic #1920 Feature parity with LoopBack 3.x (and the lack of it) where we are tracking various LB3 bits not available in LB4 yet. It's probably best to view epics via ZenHub to see all GH issues assigned to the epic. You can either install ZenHub browser extension or use their web view here: https://app.zenhub.com/workspaces/loopback-54ebffa9e8323184150c2125/issues/strongloop/loopback-next/1920.

Data validation. Currently it is divided in multiple levels REST, controller, ORM, etc... It feels like you have to use different decorators to tackle all levels of validation. It would be great if we could for example provide a common required property validator in the model along with its type and this should be implied on the ORM and REST level etc... Also sync and input-only validations such as the JSON Schema validations could be shared by the ORM and REST layer. Custom - Extra validations should be implemented when DB access is needed etc... Also it is not clear to me whether custom validations in the ORM level such as validate and validateAsync from LB3 is supported. If not, I believe custom validations in the ORM level is also a needed feature.

We have been discussing validation in different threads in the past. Epic: Validation at Model/ORM level #1872 (ZenHub view: https://app.zenhub.com/workspaces/loopback-54ebffa9e8323184150c2125/issues/strongloop/loopback-next/1872) seems to be the closest place to this topic, I think it may be better to open a new issue to discuss the feature request for validators shared by both REST and ORM layers. I remember I was commenting on that in the past, but cannot find that thread :(_

Possibly related: Epic: ENUM type (ORM, OpenAPI, etc.) #3033 (ZenHub view: https://app.zenhub.com/workspaces/loopback-54ebffa9e8323184150c2125/issues/strongloop/loopback-next/3033)

Reduce decorator boilerplate. For a newcomer to LB4 and also Typescript (such as myself 😋) dealing with all the extra info with the decorators is a bit overwhelming as well... It would be great if then OpenAPI decorators providing the response types could default to the returned response type of the decorated controller method (in similar way such as input is with @param decorator) and only provide the extra result spec if the default does not meet our requirements.

We have been discussing how to simplify response specification in https://github.com/strongloop/loopback-next/issues/1672, some improvements were implemented by https://github.com/strongloop/loopback-next/pull/4550 and there is a related issue https://github.com/strongloop/loopback-next/issues/5149 waiting for somebody to implement. @mitsos1os What do you think, is the @oas.response decorator a good solution, or are you looking for something else? Perhaps we just need to improve our documentation (and possibly the example apps?) to make @oas.response easier to find?

We have been also discussing how to simplify RequestBody annotation, see Syntactic sugar for requestBody() with custom schema options #3257.

Phew, that was a long comment! 😅 I really appreciate your honest feedback, @mitsos1os. I tried to add as much context as I could. To make this discussion actionable, I think it would be best to discuss individual items in individual threads - typically in topic-specific GitHub issues. Otherwise we risk derailing the discussion in this issue from the original goal, which was to reorganize code & docs along abstraction levels.

If you have any more suggestions that are related to our documentation and don't have their own GH issue yet, then feel free to post them to #5113.

bajtos commented 4 years ago

The discussion about abstraction levels is done, I am closing this issue as resolved.