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

Webpack 5 watch not failing on error #1204

Closed
webpack-bot opened this issue Nov 2, 2020 · 15 comments
Closed

Webpack 5 watch not failing on error #1204

webpack-bot opened this issue Nov 2, 2020 · 15 comments

Comments

@webpack-bot
Copy link

Bug report

What is the current behavior?

When I start webpack in watch mode it does not fail on a TypeScript error.

If the current behavior is a bug, please provide the steps to reproduce.

  1. Clone this GitHub repository: https://github.com/oliverschwendener/webpack-typescript
  2. Install dependencies: $ yarn install
  3. Start bundler in watch mode: $ yarn bundle --watch
  4. Look at bundler output. Everything should be OK.
[webpack-cli] Compilation starting...
[webpack-cli] Compilation finished
asset index.js 41 bytes [compared for emit] [minimized] (name: main)
./src/index.ts 49 bytes [built] [code generated]
webpack 5.3.2 compiled successfully in 1551 ms
[webpack-cli] watching files for updates...
  1. Add a code snippet to src/index.ts that should lead to a TypeScript error:
const x: string = 1243; // <= Type 'number' is not assignable to type 'string'.ts(2322)
console.log(x);
  1. Look at bundler output:
[webpack-cli] Compilation starting...
[webpack-cli] Compilation finished
asset index.js 41 bytes [emitted] [minimized] (name: main)
./src/index.ts 49 bytes [built] [code generated]
webpack 5.3.2 compiled successfully in 326 ms
[webpack-cli] watching files for updates...

What is the expected behavior?

I expect webpack to fail when trying to bundle my files when typescript/ts-loader detects an error. This worked fine so far when I was using webpack 4.44.2 but since I upgraded to webpack 5.3.2 this problem occurs. Interestingly this only happens when changing files after I started the watch process. When I add the failing code snippet before starting the watch process it fails as expected with the according error message:

[webpack-cli] Compilation finished
assets by status 41 bytes [cached] 1 asset
./src/index.ts 49 bytes [built] [code generated] [1 error]

ERROR in C:\Users\Oliver\projects\webpack-typescript\src\index.ts
./src/index.ts
[tsl] ERROR in C:\Users\Oliver\projects\webpack-typescript\src\index.ts(1,7)
      TS2322: Type 'number' is not assignable to type 'string'.

webpack 5.3.2 compiled with 1 error in 1554 ms
[webpack-cli] watching files for updates...

Other relevant information:
webpack version: 5.3.2
Node.js version: 14.14.0
Operating System: Windows 10 1909 (also reproducible on macOS 10.15 Catalina)
Additional tools: webpack-cli 4.1.0, ts-loader 8.0.7, typescript: 4.0.5


This issue was moved from webpack/webpack#11889 by @evilebottnawi. Original issue was by @oliverschwendener.

@alexander-akait
Copy link
Contributor

Found it high priority

@johnnyreilly
Copy link
Member

Thanks for the detailed report - I can't dig into this myself right now, but you'll help anyone that comes after and I'd encourage you to dig into this also.

One thing that occurs to me is that it looks like you're running webpack with watch but mode is not set and so is production by default. Typically you'd be running in development mode with watch.

https://webpack.js.org/configuration/mode/

Not sure if it's definitely relevant but I thought I'd share

@oliverschwendener
Copy link

Ah yes you're right, I forgot to specify mode. I tried it with mode set to development but the same issue still occurs.

@johnnyreilly
Copy link
Member

johnnyreilly commented Nov 3, 2020

Interesting - I wonder if this is linked to the work @sanex3339 did around deprecation warnings: #1195 (this PR was specifically related to the adding of errors to webpack)

@appzuka has also done some work on deprecation warnings but I don't think that's related #1200

If anyone would like to investigate that would be greatly appreciated.

@appzuka
Copy link
Member

appzuka commented Nov 4, 2020

This seems to be caused by my change to stop the deprecation warnings when declarations is true #1200.

Calling makeAfterCompile during the afterCompile hook causes warnings as it is no longer allowed to add assets (such as declarations) at this stage. But calling it in the afterProcessAssets hook fails to add errors when in watch mode.

I'm thinking that the solution may be to split makeAfterCompile into 2 parts, separating out the calls which add assets and those which don't (such as reporting errors to webpack). I'll try to submit a PR as soon as I have time.

@johnnyreilly
Copy link
Member

Thanks to @appzuka's fine work this should be resolved with https://github.com/TypeStrong/ts-loader/releases/tag/v8.0.10

@oliverschwendener
Copy link

Thanks for your efforts guys! Highly appreciated. Works as expected in 8.0.10 👍

@oliverschwendener
Copy link

I guess this issue can be closed now.

@dl748
Copy link

dl748 commented Feb 8, 2021

This looks like its been reintroduced. I'm getting this exact behavior on these versions

webpack 5.21.2
webpack-cli 4.5.0
webpack-dev-server 3.11.2
ts-loader 8.0.15
node 14.15.4

@appzuka
Copy link
Member

appzuka commented Feb 8, 2021

@dl748, I can verify the problem and have a solution. I have already changed this section of code a few times and each change I make has unexpected consequences so I have gone back to understand why this has happened and whether changing it again will break something else.

Originally, declaration file and errors were emitted during the compiler.afterCompile hook. In webpack5 it is no longer allowed to emit assets in the afterCompile hook so I moved this to the compilation.afterProcessAssets hook. But then as reported here errors were not emitted in watch mode. So I made the change here which is to emit errors during the compiler.afterCompile hook but to emit declaration files in the loader._compilation.afterProcessAssets hook.

But then it was reported that declaration files are not emitted in subsequent runs during watch mode. This was traced to loader._compilation not being persistent so we need to use a compilation hook to get the compilation so we can add the afterProcessAssets hook. With hindsight, this was also the cause of errors not being emitted in the previous step and would have been a better solution.

But now I have broken error reporting again. I can see this is because when we emit the declaration files the list of files to check for errors is reset, so when we check whether there are any errors in the afterCompile stage no files are checked.

If I change the code so that errors and declaration files are emitted at the compiler.afterProcessAssets stage everything seems to work correctly for both errors and declaration files for both normal and watch builds. I'll raise a PR for this change.

@johnnyreilly
Copy link
Member

https://github.com/TypeStrong/ts-loader/releases/tag/v8.0.16 - shipped

@dl748
Copy link

dl748 commented Feb 8, 2021

Will have to wait for the npm module release, unable to build v8.0.16

> ts-loader@8.0.16 build /repos/ts-loader
> tsc --version && tsc --project "./src"

Version 4.1.3
src/utils.ts:121:38 - error TS2769: No overload matches this call.
  Overload 1 of 3, '(path: string | number | Buffer | URL, options?: { encoding?: null | undefined; flag?: string | undefined; } | null | undefined): Buffer', gave the following error.
    Argument of type 'string' is not assignable to parameter of type '{ encoding?: null | undefined; flag?: string | undefined; } | null | undefined'.
  Overload 2 of 3, '(path: string | number | Buffer | URL, options: "ascii" | "utf8" | "utf-8" | "utf16le" | "ucs2" | "ucs-2" | "base64" | "latin1" | "binary" | "hex" | { encoding: BufferEncoding; flag?: string | undefined; }): string', gave the following error.
    Argument of type 'string' is not assignable to parameter of type '"ascii" | "utf8" | "utf-8" | "utf16le" | "ucs2" | "ucs-2" | "base64" | "latin1" | "binary" | "hex" | { encoding: BufferEncoding; flag?: string | undefined; }'.
  Overload 3 of 3, '(path: string | number | Buffer | URL, options?: "ascii" | "utf8" | "utf-8" | "utf16le" | "ucs2" | "ucs-2" | "base64" | "latin1" | "binary" | "hex" | (BaseEncodingOptions & { ...; }) | null | undefined): string | Buffer', gave the following error.
    Argument of type 'string' is not assignable to parameter of type '"ascii" | "utf8" | "utf-8" | "utf16le" | "ucs2" | "ucs-2" | "base64" | "latin1" | "binary" | "hex" | (BaseEncodingOptions & { flag?: string | undefined; }) | null | undefined'.

121     return fs.readFileSync(fileName, encoding);
                                         ~~~~~~~~

@dl748
Copy link

dl748 commented Feb 8, 2021

ts-loader@8.0.16 from npmjs.com is working as expected @appzuka @johnnyreilly

@josfaber
Copy link

@appzuka @johnnyreilly Here's the second confirm that upgrading from ts-loader@8.0.15 to ts-loader@8.0.16 fixed this issue (kudos to ncu).

This was referenced Mar 14, 2021
@leppaott
Copy link

leppaott commented Jun 20, 2022

Still seems to be happening? But could be nodemon plugin as well that is just restarting the server...

    "ts-loader": "^9.3.0",
    "webpack": "^5.73.0",

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

No branches or pull requests

8 participants