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

Move @types/webpack to dependencies rather than devDependencies #127

Open
rdsedmundo opened this issue Apr 17, 2019 · 24 comments · May be fixed by #165
Open

Move @types/webpack to dependencies rather than devDependencies #127

rdsedmundo opened this issue Apr 17, 2019 · 24 comments · May be fixed by #165

Comments

@rdsedmundo
Copy link

node_modules/clean-webpack-plugin/dist/clean-webpack-plugin.d.ts:1:33 - error TS7016: Could not find a declaration file for module 'webpack'. '/node_modules/webpack/lib/webpack.js' implicitly has an 'any' type.
  Try `npm install @types/webpack` if it exists or add a new declaration (.d.ts) file containing `declare module 'webpack';`

1 import { Compiler, Stats } from 'webpack';

I'm getting this error even though I'm not using typings on my Webpack configs at all, so it shouldn't be happening. I know I can just install the @types/webpack myself, but since the package is using it for something, it actually should be included as a dependency and installed automatically once I get the package.

From TS docs:

For that reason, we used "dependencies" and not "devDependencies", because otherwise our consumers would have needed to manually install those packages. If we had just written a command line application and not expected our package to be used as a library, we might have used devDependencies.

@chrisblossom
Copy link
Collaborator

The reason is because you should be installing @types/webpack in a typescript project with webpack.

Please provide a use case that you would install clean-webpack-plugin, but not webpack.

@rdsedmundo
Copy link
Author

I'm not writing my webpack configs in TypeScript, so I don't see the reason why I should mandatorily add it as a dependency of my project.

@chrisblossom
Copy link
Collaborator

You most likely are using the allowJs and/or checkJs option, which you'd still need @types/webpack. That or set webpack to ignore your webpack config

@chrisblossom
Copy link
Collaborator

Moved @types/webpack to dependencies in #136.

@Sebazzz
Copy link

Sebazzz commented Jun 2, 2019

This change can actually break the build for web projects, as @types/node are now pulled into the scope causing jquery bindings to break among others.

@chrisblossom
Copy link
Collaborator

@Sebazzz please provide a repository showing this and/or link showing why/when this could happen.

@Sebazzz
Copy link

Sebazzz commented Jun 3, 2019

I don't have a minimal repo yet, but you can check out this project and update the clean-webpack-plugin package (yarn). The additional typings will now be taken into consideration for compilation.

@rdsedmundo
Copy link
Author

I don't see how this will break any projects. Mine is also a Web project and it worked fine. Can you at least send the jQuery errors that were thrown?

@chrisblossom chrisblossom reopened this Jun 3, 2019
@chrisblossom
Copy link
Collaborator

This is definitely causing a problem, but am unsure what to do to fix it.

@types/webpack-env fixes some of the problems.

I think we probably need to remove the @types/webpack dependency and then document that you might need to install it manually, as well as @types/webpack-env.

Thoughts?

@rdsedmundo
Copy link
Author

What's the error that's happening?

If someone is experimenting an error because it's now installed automatically, they will keep facing it if you remove and then they install it manually (on a project that uses TypeScript).

It's better to understand and try to fix the error (if it's possible) rather than shallowing it.

@chrisblossom
Copy link
Collaborator

chrisblossom commented Jun 11, 2019

If someone is experimenting an error because it's now installed automatically, they will keep facing it if you remove and then they install it manually (on a project that uses TypeScript).

I agree, but I think it has to be solved on a project by project basis. The issue seems that @types/webpack brings in @types/node, which causes some issues, especially with front-end only applications.

It's better to understand and try to fix the error (if it's possible) rather than shallowing it.

Again, I agree. But I also don't think pulling in this library should cause additional typescript errors otherwise not present.

If anyone knows someone that is well-versed in Webpack with Typescript that we can get in this thread it would be great.

With the example provided by @Sebazzz:

Without @types/webpack-env:

js/App/App.ts:14:16 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

14     if (module.hot) {
                  ~~~

js/App/App.ts:15:16 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

15         module.hot.accept('./BindingHandlers/All', () => {
                  ~~~

js/App/Navigation.ts:8:16 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

8     if (module.hot) {
                 ~~~

js/App/Navigation.ts:9:16 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

9         module.hot.accept('App/Pages', () => {
                 ~~~

js/AppFramework/AppFactory.ts:223:16 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

223     if (module.hot) {
                   ~~~

js/AppFramework/AppFactory.ts:224:16 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

224         module.hot.accept('./BindingHandlers/All', () => {
                   ~~~

js/AppFramework/Components/LoadingBar.ts:171:31 - error TS2558: Expected 0 type arguments, but got 1.

171     public template = require<string>('./templates/loading-bar.html');
                                  ~~~~~~

js/AppFramework/Components/Modal.ts:256:20 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

256         if (module.hot) {
                       ~~~

js/AppFramework/Components/Modal.ts:257:20 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

257             module.hot.accept('./templates/modal.html', () => {
                       ~~~

js/AppFramework/Components/Popover.ts:58:20 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

58         if (module.hot && !this.template) {
                      ~~~

js/AppFramework/Components/Popover.ts:59:20 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

59             module.hot.accept('./templates/popover.html', () => {
                      ~~~

js/AppFramework/Forms/Confirmation.ts:64:12 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

64 if (module.hot) {
              ~~~

js/AppFramework/Forms/Confirmation.ts:65:12 - error TS2339: Property 'hot' does not exist on type 'NodeModule'.

65     module.hot.accept('./templates/confirmation.html');
              ~~~

With @types/webpack-env:

js/AppFramework/ServerApi/HttpClient.ts:78:28 - error TS2345: Argument of type 'jqXHR<any>' is not assignable to parameter of type 'Promise<T>'.
  Property 'finally' is missing in type 'jqXHR<any>' but required in type 'Promise<T>'.

78             requestHandler(promise);
                              ~~~~~~~

  node_modules/typescript/lib/lib.es2018.promise.d.ts:31:5
    31     finally(onfinally?: (() => void) | undefined | null): Promise<T>
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    'finally' is declared here.

js/AppFramework/ServerApi/HttpClient.ts:81:9 - error TS2322: Type 'jqXHR<any>' is not assignable to type 'Promise<T>'.

81         return promise;
           ~~~~~~~~~~~~~~~

I am unsure how to solve the jQuery issues.

@rdsedmundo
Copy link
Author

The problem here is at his code. He's using jQuery Promises but he's declaring them as natives Promises. It looks like their interfaces are not compatible anymore with the introduction of the finally, I suppose jQuery either doesn't support or that or its type definitions weren't updated if the support was added.

He should change those declarations here:

    public get<T>(url: string, data: any = null): Promise<T> {
        return this.method(url, 'GET', data);
    }

    public getText(url: string, data: any = null): Promise<string> {
        return this.method(url, 'GET', data, textRequest);
    }

    public put<T>(url: string, data: any = null): Promise<T> {
        return this.method(url, 'PUT', data);
    }

    public post<T>(url: string, data: any = null): Promise<T> {
        return this.method(url, 'POST', data);
    }

    public delete<T>(url: string, data: any = null): Promise<T> {
        return this.method(url, 'DELETE', data);
    }

to use JQuery. jqXHR<T> instead of Promise<T>.

@Sebazzz
Copy link

Sebazzz commented Jun 12, 2019

jQuery 3+ promises are actually compatible with ES promises, or at least, they were. You also may, for instance, await jquery promises.

@rdsedmundo
Copy link
Author

See: jquery/jquery#4290

So they've decided to not implement finally, therefore they are not compatible anymore with ES2018+. I'd suggest you commenting on this issue or opening another there with your use case.

@chrisblossom
Copy link
Collaborator

Thanks @rdsedmundo for tracking down the jquery issue!

This still leaves us with the @types/webpack-env issue. I think adding a note to the readme should be enough. What does everyone think?

@Sebazzz
Copy link

Sebazzz commented Jun 13, 2019

That seems like a logical solution.

@pshurygin
Copy link

This is definitely a breaking change, and a big one. I have just updated clean-webpack-plugin to version 3, which caused @types/node being installed into node_modules, which in turn caused million of typescript errors in our angular/typescript project, because now we had two type declarations for setTimeout and other browser methods.

The issue was really hard to investigate, as you wouldn't think that updating a plugin like this can cause these weird type errors across your project.

And the only workaround available is downgrading plugin version back to 2.x

@chrisblossom
Copy link
Collaborator

@pshurygin Have you installed @types/webpack-env? If so, what are the errors afterwards?

@pshurygin
Copy link

Just tried to install it, but it changes nothing.
plugin v2 + @types/webpack-env = no errors
plugin v3 + @types/webpack-env = errors

As it was mentioned in this thread, the issue is that @types/webpack, which is brought by v3, brings @types/node alongside self, which leads to errors in browsed-based typescript apps(conflicting type definitions between node and browser). Unless you remove it from dependencies, the issue would persist.
Correct me if i'm wrong but i think that neither @types/webpack nor @types/webpack-env should be included in dependencies, because they are not required for the functioning of this plugin. I'm not sure why the OP had this issue, but this was obviously not the right solution for it.

To be clear, our webpack.config is native javascript, so we are not using @types/webpack or @types/webpack-env ourselves. Also we are not using allowJs option.

@rdsedmundo
Copy link
Author

I'd say to revert this if it's causing too many problems.

I have the allowJs option turned on, although my webpack config is also on VanillaJS.

@phil-lgr
Copy link

phil-lgr commented Oct 7, 2019

the 'webpack' package has its own declaration files, '@types/webpack' should not be used anymore, this has been the case for months if not years

image

https://github.com/webpack/webpack/tree/master/declarations

please consider removing '@types/webpack' package and depending on 'webpack' only

@phil-lgr
Copy link

phil-lgr commented Oct 7, 2019

@chrisblossom
Copy link
Collaborator

I get the error Could not find a declaration file for module 'webpack'. without @types/webpack.

@chrisblossom chrisblossom linked a pull request Oct 16, 2019 that will close this issue
@fwh1990
Copy link

fwh1990 commented Oct 19, 2020

Try to remove @types/webpack for webpack@5

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

Successfully merging a pull request may close this issue.

6 participants