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

Expose more of the internal API #2292

Closed
fwoelffel opened this issue May 28, 2019 · 6 comments
Closed

Expose more of the internal API #2292

fwoelffel opened this issue May 28, 2019 · 6 comments

Comments

@fwoelffel
Copy link
Contributor

fwoelffel commented May 28, 2019

Feature Request

Is your feature request related to a problem? Please describe.

This feature request isn't really related to a problem but more like a concern about the support of what looks like some internal API. As I'd like to keep this short, I'll dive directly into the technical details. I'm using the InstanceWrapper and Module interfaces from the injector subdirectory of the core package. Those two elements are not exposed through the package root barrel file, and the imports look like this:

import { InstanceWrapper } from '@nestjs/core/injector/instance-wrapper';
import { Module } from '@nestjs/core/injector/module';

I'm wondering if it is safe to use these imports since those are not exposed at the core's root.

The same goes for:

If you feel like you need more concrete details, have a look at this class.

Certainly, others would like to have some more internals exposed. Maybe this issue would be the right place to build a list of what could/should be exposed.

Describe the solution you'd like

My proposal is to simply expose more of the internal API, as long as the newly expose elements are considered stable.

What is the motivation / use case for changing the behavior?

This would allow the community to get a better grasp to NestJS' internals and write better and more robust integrations.


English is obviously not my first language. If, in any case, you feel like you can't completely understand what I ask for, don't hesitate to tell me and I'll do my best to rewrite this issue.

@fwoelffel fwoelffel added needs triage This issue has not been looked into type: enhancement 🐺 labels May 28, 2019
@BrunnerLivio
Copy link
Member

I'm wondering if it is safe to use these imports since those are not exposed at the core's root.

No, it is not safe to use this. If we would change these elements it would not be declared as breaking change and we will not give further support in case you application breaks because of it.


Nonetheless what you are requesting is definitely mandatory for Nest. I think it would be interesting to compile a list of internals, which we should expose or make similar functionality part of the public API.

What comes to my mind:

@kamilmysliwiec kamilmysliwiec removed the needs triage This issue has not been looked into label Jul 1, 2019
@marcus-sa
Copy link

marcus-sa commented Aug 14, 2019

I think we need to discuss this further.
If we have to utilize Bazel properly we need to have a structure similar to https://github.com/angular/angular/tree/master/packages/common as opposed to our current https://github.com/nestjs/nest/tree/master/packages/common

@Toxicable
Copy link

You don't have to use ng_package and therefore have a clear distinction between public and private apis with bazel.
ng_package is primarily focused towards frontend applications where the you give the bundler multiple options on what format to use.
With Nodejs packages ts_library -> npm_package makes a bunch more sense and should maintain the current api that exists in this repository

@marcus-sa
Copy link

marcus-sa commented Aug 20, 2019

@Toxicable the use case is the exact same as http in the common package
You don't see Angular export their http module in their common module's public api?
It doesn't make sense for us to do so either, since IDE's will resolve down to the @nestjs/common/http path instead.

#2772

@kamilmysliwiec
Copy link
Member

Most of them (MetadataScanner, Type, ClassProvider, ValueProvider, FactoryProvider etc) are now exported from root

@lock
Copy link

lock bot commented May 5, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants