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

Remove types from dependencies #82

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

stephane-segning
Copy link

Because this is part of the main dependencies, the overall package is very huge. This PR do fixes that

@stegano
Copy link
Owner

stegano commented Mar 19, 2023

Hi @stephane-segning

Thanks for your PR

Could you please check issue #17?
(If you move the @types/http-proxydependency to the devDependencies field, the TS compiler will not properly interpret the NextHttpProxyMiddlewareOptions interface.)

Thanks 😀

@stephane-segning
Copy link
Author

I've read your comment, @stegano. The thing is, that the error might come from the way it is compiled, not from the dependencies. I'm trying to provide an example of fix in the PR itself.

@stephane-segning
Copy link
Author

To help me fix it, I was worknig on this other library TrySound/rollup-plugin-uglify#91

@stegano
Copy link
Owner

stegano commented Mar 24, 2023

Hi @stephane-segning
I'm sorry but I don't quite understand. Can you explain what part is the problem and what solved it?

If the user installs the next-http-proxy-middleware library, the @types/http-proxy dependency library must also be installed.

If you remove @types/http-proxy from the dependencies of this project, When user install next-http-proxy-middleware , @types/http-proxy must be installed separately. Do you think this is correct?

@stephane-segning
Copy link
Author

stephane-segning commented Mar 26, 2023

The problem is the TS config. I've updated it quite a little bit. And also, you should include all *.d.ts files inside the final release of your app.

The lib @types/http-proxy should always be installed if a user is working with dev deps. Typically, in dev env.

If we put @types/http-proxy in dev-deps, the problem is solved. Now, if the client has any problems with it, they explain them to us and we solve them accordingly. But not by adding @types/* to the main deps.

@stegano
Copy link
Owner

stegano commented Mar 26, 2023

The problem is the TS config. I've updated it quite a little bit. And also, you should include all *.d.ts files inside the final release of your app.

The lib @types/http-proxy should always be installed if a user is working with dev deps. Typically, in dev env.

If we put @types/http-proxy in dev-deps, the problem is solved. Now, if the client has any problems with it, they explain them to us and we solve them accordingly. But not by adding @types/* to the main deps.

I'm sorry but I still don't quite understand,

  1. @types/http-proxy is a dependency module that must be installed to use this library. I don't understand why the user has to install this separately. If a user using this library already has another version of the http-proxy library installed, the user will be confused because they have to consider two versions of @types/http-proxy.

  2. Even if a user makes a product using this library, all typing (interface) files will be removed from the build result.

Please let me know if I'm misunderstanding something.

@stegano
Copy link
Owner

stegano commented Mar 26, 2023

The problem is the TS config. I've updated it quite a little bit. And also, you should include all *.d.ts files inside the final release of your app.

If you need to include all *.d.ts files(http-proxy, etc..) in the final release of your app, you should also include the original source code of the http-proxy library that this library depends on. This isn't common.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants