Loopback-next: Consistency with re-exports (route decorators, `@inject` etc.)

Created on 12 Mar 2018  路  9Comments  路  Source: strongloop/loopback-next

Description

There are a couple of modules which export the same functions, presumably for the sake of backwards compatibility. I wanted to create this issue to discuss, for the sake of consistency, where we should import functions like these from in our documentation/code. For example, should we be using @get from @loopback/rest or @openapi-v3?

In addition to this, I also want to talk about whether we should have these re-exports at all in the first place.

Things to think about:

  • What has been re-exported for backwards compatibility with our own code?

    • How long do we plan on supporting this backwards compatibility? When do we plan on getting rid of these?

  • For the re-exports in question, which one do we advocate our users and ourselves to use?

    • everything from @loopback/openapi-v3 (@get, @param, etc.)

    • @loopback/openapi-v3

    • @loopback/rest

    • everything from @loopback/metadata

    • @loopback/metadata

    • @loopback/context

    • @inject, Context

    • @loopback/context

    • @loopback/core

Acceptance Criteria

  • [ ] change examples in the monorepo (and the CLI template) and the docs to export the openapi-v3 decorators from rest instead of openapi-v3

    • [ ] make appropriate changes to package.json if needed

  • [ ] make sure our codes or docs are not being redundant. For example, making sure that a package doesn't use @loopback/metadata when it already uses @loopback/context (same for context/core and openapi-v3/rest)
  • [ ] document that we reexport openapi-v3 from rest, metadata from context, and @inject and context from core under their READMEs
  • [ ] Run lerna command to see a dependency graph of other examples of these dependency links, if there are any, decide on what the 'best practice' is.
  • [ ] Review existing packages for potential reexports (or deletion of) of other children dependencies

    • [ ] If any, create issues for it; DO NOT work on the reexport/re-documentation effort

Docs Examples stale tech-debt

Most helpful comment

I agree that LB4 consumers (app and extension authors) should be importing REST-related decorators from @loopback/rest.

We need further discussion for using @inject from core vs context and context vs metadata. We could create a separate issue for discussion on those topics and have just the rest vs openapi-v3 issue up for estimation.

Well, is the effort required to groom, estimate and plan those follow-up issues worth it? I'd personally prefer to fix all three topic in this single story.

In my opinion, people should import symbols from the highest-level package they are depending on.

  • Extensions that do not require any functionality from @loopback/core should depend on @loopback/context only and load @inject from @loopback/context. This will allow a single version of an extension to support multiple semver-major versions of loopback core, to the extent of semver-major changes not affecting the parts used by the extension.

  • Applications should not depend on a specific version of @loopback/context, they should always import @inject from @loopback/core.

  • If there is an extension that requires @loopback/core as a dependency, then it should import @inject from @loopback/core and NOT list context as a dependency.

Same principle applies to context vs metadata:

  • If an extension does not require any context API and relies on metadata only, then it should import things from metadata package. This allows the extension to support multiple semver-major versions of context and core .
  • If the extension requires context API, then it should depend on context only (no metadata dependency) and import symbols from context.

I think it's important to capture these guidelines in our documentation, maybe create a new page in "best practices" section. This task needs to be added to the acceptance criteria.

All 9 comments

I don't think any of the openapi-v3 packages should be used directly from @loopback/openapi-v3. @loopback/openapi-v3 helps us under the hood for routing and more but a user doesn't need to be aware of this and as such shouldn't have to use this package directly, rest is all the user should need to interact with. Plus it's kind of why we re-export those decorators imo.

cc @bajtos

Export operation decorators from rest sounds good to me, and that's the way we suggest in the documentation.

We need further discussion for using @inject from core vs context and context vs metadata. We could create a separate issue for discussion on those topics and have just the rest vs openapi-v3 issue up for estimation.

I agree that LB4 consumers (app and extension authors) should be importing REST-related decorators from @loopback/rest.

We need further discussion for using @inject from core vs context and context vs metadata. We could create a separate issue for discussion on those topics and have just the rest vs openapi-v3 issue up for estimation.

Well, is the effort required to groom, estimate and plan those follow-up issues worth it? I'd personally prefer to fix all three topic in this single story.

In my opinion, people should import symbols from the highest-level package they are depending on.

  • Extensions that do not require any functionality from @loopback/core should depend on @loopback/context only and load @inject from @loopback/context. This will allow a single version of an extension to support multiple semver-major versions of loopback core, to the extent of semver-major changes not affecting the parts used by the extension.

  • Applications should not depend on a specific version of @loopback/context, they should always import @inject from @loopback/core.

  • If there is an extension that requires @loopback/core as a dependency, then it should import @inject from @loopback/core and NOT list context as a dependency.

Same principle applies to context vs metadata:

  • If an extension does not require any context API and relies on metadata only, then it should import things from metadata package. This allows the extension to support multiple semver-major versions of context and core .
  • If the extension requires context API, then it should depend on context only (no metadata dependency) and import symbols from context.

I think it's important to capture these guidelines in our documentation, maybe create a new page in "best practices" section. This task needs to be added to the acceptance criteria.

@bajtos I agree with what you've proposed there and will be updating the acceptance criteria to reflect the proposal if I can get more :+1:s from the others.

I like the proposal BUT it seems like a big if this ... import from ... else if that ... import from ... which can be confusing to read and make sense of as a best practice. What happens if you start writing an app / extension thinking you don't need core but later end up needing core?

I think this ticket should only be used to make sure our code / examples / docs follow these best practices but I don't think we need to explicitly document them except the fact that core re-exports context, etc ... so users aren't confused about how inject can be imported from either package.

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

Was this page helpful?
0 / 5 - 0 ratings