Definition files are not updated after initial build when using watch mode in Webpack 5.

[JonWallsten]

Expected Behaviour

When using watch mode in Webpack 5 and I'm changing a file, the index.js and index.d.ts bundle files should be updated with the new code/definitions and I should be able to import the code in my app that consumes the code.

Actual Behaviour

Only the index.js file is updated when adding new code. All d.ts files as well as the index.d.ts bundle remains untouched and I get compiler error when importing the new code (in runtime the code actually runs fine).

Steps to Reproduce the Problem

1. Run Webpack 5 in watch mode: node node_modules\webpack\bin\webpack.js --config config/webpack.dev.ts --watch
2. Change any code in the project
3. Check the dist folder and compare the times between the .js and the d.ts files

Location of a Minimal Repository that Demonstrates the Issue.

(Working on it)

webpack.dev.ts

module: {
    rules: [
        {
            test: /\.ts$/,
            loader: 'ts-loader',
            options: {
                configFile: '../tsconfig.build.json'
            },
            include: [
                projectRootPath('typings')
                rootPath('src'),
                rootPath('typings')
]
        }
    ]
},

tsconfig.build.json

{
  "extends": "../../tsconfig.json",
  "compilerOptions": {
    "rootDir": "./src",
    "declaration": true,
    "declarationMap": true,
    "declarationDir": "./dist",
    "outDir": "./dist"
  },
  "files": [
    "./src/index.ts",
    "./typings/global.d.ts",
    "../../typings/global.d.ts"
  ]
}

tsconfig.json

{
  "compilerOptions": {
    "rootDir": ".",
    "baseUrl": ".",
    "target": "es5",
    "module": "commonjs",
    "moduleResolution": "node",
    "allowSyntheticDefaultImports": true,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "removeComments": false,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noEmitHelpers": true,
    "noEmitOnError": true,
    "importHelpers": true,
    "noImplicitAny": false,
    "skipLibCheck": true,
    "strictNullChecks": false,
    "strictPropertyInitialization": false,
    "pretty": true,
    "sourceMap": true,
    "declaration": false,
    "preserveConstEnums": true,
    "downlevelIteration": true,
    "lib": [
        "dom",
        "es2015",
        "es2017.object",
        "es2016.array.include",
        "es2017.string",
        "es2018.promise",
        "es2019.string"
    ],
    "paths": {
      "@oas/web-lib-core": ["./packages/web-lib-core/dist"],
      "@oas/web-lib-common": ["./packages/web-lib-common/dist"],
      "@oas/web-lib-angular-js": ["./packages/web-lib-angular-js/dist"],
      "@oas/web-lib-angular": ["./packages/web-lib-angular/dist"],
      "@oas/internal-lib-e2e": ["./packages/internal-lib-e2e/dist"]
    }
  },
  "compileOnSave": false,
  "buildOnSave": false
}

I'm open for the possibility that this bug is not an issue for this project. But it is my guess.

[JonWallsten]

@johnnyreilly: I hope everything is fine! Can you help me point to where in the loader the definition files are generated? I can only find the source code and source maps.

[johnnyreilly]

If memory serves it comes from here:

ts-loader/src/instances.ts

Line 798 in 268bc69

if (fileExtensionIs(filePath, instance.compiler.Extension.Dts)) {

[JonWallsten]

@johnnyreilly: Thanks! I'm trying to figure out how this works with the Webpack 5. It's not like I remember it.

I can see that the d.ts file is part of the the outputFiles array:

I can also see that it's content is correct:

So I'm not sure where it's lost or who's ignoring the changes.

Edit: Why is it returning an empty array?

if (fileExtensionIs(filePath, instance.compiler.Extension.Dts)) {
  return [];
}

Edit 2: That is irrelevant it seems. Generated d.ts files doesn't seem to be handled by getEmitOutput.

Edit 3: getEmit is where I get lost:

The index.d.ts file is filtered away here:

So I guess they are handles elsewhere? I can't find where the d.ts files are being fed back to Webpack.

[johnnyreilly]

I can't find where the d.ts files are being fed back to Webpack.

They'll be coming back from the getEmit from TypeScript I think - sorry, on my phone so hard to help

[JonWallsten]

I can't find where the d.ts files are being fed back to Webpack.

They'll be coming back from the getEmit from TypeScript I think - sorry, on my phone so hard to help

No worries. I'll continue to dig a bit. I really can't find where they are processed.
It's pretty obvious in Webpack 4:

But I can't find the corresponding code for Webpack 5.

Edit: Oh, I see. It is using the same code for Webpack 5, but it's only called the first build. For the subsequent builds addAssets are false.

The thing is, it never enters here in the subsequent builds:

Can it be that Webpack never triggers afterProcessAssets hook on rebuilds in this case?

[JonWallsten]

Tried to implemen the processAssets hook instead:

It is never called either on subsequent builds. The afterCompile hook registered a few lines prior is.

[JonWallsten]

@johnnyreilly: Finally got it to work with help from https://github.com/alexander-akait
We need to figure out how to do this is a nice way though.
The issue is that the compilation is not persistent, so you need to subscribe to loader._compiler.hooks.compilation to get the new instances. However, when we call addAssetHooks it seems to be to late already to subscribe to the first one. So I made a POC where I add the first one manually, and the subsequent calls are made from the compilation hook.

const callback = (compilation) => {
    compilation.hooks.processAssets.tap({ name: 'ts-loader', stage: webpack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL}, (_assets) => {
        after_compile_1.makeAfterCompile(instance, true, false, instance.configFilePath)(compilation, () => {
            return null;
        });
    });
};

// For future calls
loader._compiler.hooks.compilation.tap('ts-loader', callback);
// For first call
callback(loader._compilation);

[johnnyreilly]

Thanks for reporting back - that's super helpful. Thanks also to @alexander-akait. Your approach is interesting... Not too sure how I feel about it. Would it affect just the functionality around .d.ts or everything? cc @appzuka in case you're intrigued.

[JonWallsten]

@johnnyreilly: The only real change here is that the hook now is actually called every build, not just the initial one. So I guess it should mostly solve things! ;)
But I think the current way of adding the assets is no longer correct.
See here for reference:
webpack/webpack#11425 (comment)

What seems to be the new way

compilation.emitAsset('test.js', new sources.RawSource(data))

Current way

const assetPath = path.relative(compilation.compiler.outputPath,outputFile.name);
compilation.assets[assetPath] = {
    source: () => outputFile.text,
    size: () => outputFile.text.length,
};

Also, it might be a bad idea to use loader._compilation if the compilation is not persistent.
Otherwise it obviously need to be updated in the compilation hook.

const callback = (compilation) => {
    loader._compilation = compilation;
    compilation.hooks.processAssets.tap({ name: 'ts-loader', stage: webpack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL}, (_assets) => {
        after_compile_1.makeAfterCompile(instance, true, false, instance.configFilePath)(compilation, () => {
            return null;
        });
    });
};

So maybe we can do a "quick fix" first and work on a more robust solution on the side? This bug is stopping a large PR for Webpack 5 + Angular 11 + Monorepo improvements for me. I could however patch the loader with my MonkeyPatch script locally.

Edit: It seems like others are rewriting the code and dropping support for Webpack before v4.40.0 to allow the same code to be used. It seems like this processAssets was introduced in 4.40.0. Not sure how big rewrite it is though.

[johnnyreilly]

Do you want to start work on a prospective PR which we can use as a basis for discussion / collaboration?

Dropping support for webpack before v4.40.0 is possible. It would make it a breaking changes release, but that's okay.

The only real change here is that the hook now is actually called every build, not just the initial one

I'm somewhat wary of this as the performance impact is likely to be significant.

[JonWallsten]

Do you want to start work on a prospective PR which we can use as a basis for discussion / collaboration?

I'm not sure I have the time required right now. This upgrade to Webpack 5 has been eating so much time from my project due to bug hunting. I did a monkey patch to fix the issue locally for our group for now. But I will happily look at it once I'm ahead again.

Dropping support for webpack before v4.40.0 is possible. It would make it a breaking changes release, but that's okay.

According to @sokra it should not be a huge deal: #1243 (comment)

The only real change here is that the hook now is actually called every build, not just the initial one

I'm somewhat wary of this as the performance impact is likely to be significant.

But isn't that whats already happening in Webpack 4? And was supposed to happen in Webpack 5, but didn't just because the hook was never called again? I'm not sure how heavy the process of adding/writing the definition files are. But from what I saw, even the ones not changed was written again (does not happen the initial build). So that might be something to look at. Maybe it's fixed by itself if we use the new way of adding files.

Let's start with creating a POC and see what happens. I can try to do that next week or so.

[johnnyreilly]

Cool - we appreciate your efforts. Don't worry about what you can do and when, I appreciate everything is best efforts. Myself I'm contending with home schooling right now and so my ability to focus on anything is challenged 😅

[alexander-akait]

@JonWallsten Can you put PoC here, maybe other developers found it useful and help you

[JonWallsten]

@alexander-akait: Yeah, I'll open up a draft as soon as I have the time. I will let you know if I get stuck! But I found a some example that you've written i another issue. So I'l take a look at that. Thanks for you guidance so far!

[JonWallsten]

Cool - we appreciate your efforts. Don't worry about what you can do and when, I appreciate everything is best efforts. Myself I'm contending with home schooling right now and so my ability to focus on anything is challenged 😅

Yeah, and with much at work, no time on the weekends anymore and this constant darkness it's hard to find time/motivation to do work after work! 😅

[appzuka]

Thanks for looping me in @johnnyreilly. I am intrigued and perhaps also responsible. The section of code here was one I added in #1200 and also #1208.

The changes were to stop adding assets in the afterCompile compiler hook, which is deprecated in webpack5. I could not find a suitable compiler hook so I used the afterProcessAssets compilation hook. I linked to the compilation using loader._compilation.

As others in this thread have found, in watch mode loader._compilation is not persistent. I created a minimal repository to test this and found the same as others. The repo is at:

https://github.com/appzuka/ts-loader-1243

I used the solution proposed by @JonWallsten , to add a compilation hook in the compiler and to add the processAssets hook inside that hook. (Adding a hook inside a hook feels clumsy but I cannot see a better hook to use.)

I also confirmed that the first run needs to be handled separately using loader._compilation. I tried using the compiler-compilation hook at ts-loader's entry point but this is already too late for the first run. I did confirm that only 1 of these hooks runs for each compilation run. Even if you make multiple changes in watch mode this does not result in multiple hooks running.

I will go ahead and raise a PR for this change. It is possible that in the future the new hooks available in webpack5 can be used differently.

[JonWallsten]

Thanks for looping me in @johnnyreilly. I am intrigued and perhaps also responsible. The section of code here was one I added in #1200 and also #1208.

The changes were to stop adding assets in the afterCompile compiler hook, which is deprecated in webpack5. I could not find a suitable compiler hook so I used the afterProcessAssets compilation hook. I linked to the compilation using loader._compilation.

As others in this thread have found, in watch mode loader._compilation is not persistent. I created a minimal repository to test this and found the same as others. The repo is at:

https://github.com/appzuka/ts-loader-1243

I used the solution proposed by @JonWallsten , to add a compilation hook in the compiler and to add the processAssets hook inside that hook. (Adding a hook inside a hook feels clumsy but I cannot see a better hook to use.)

I also confirmed that the first run needs to be handled separately using loader._compilation. I tried using the compiler-compilation hook at ts-loader's entry point but this is already too late for the first run. I did confirm that only 1 of these hooks runs for each compilation run. Even if you make multiple changes in watch mode this does not result in multiple hooks running.

I will go ahead and raise a PR for this change. It is possible that in the future the new hooks available in webpack5 can be used differently.

Hey @appzuka!
Thanks for joining! Did you see my comment about the change in how assets are handled in Webpack => 4.44.0?

compilation.emitAsset('test.js', new sources.RawSource(data))

We should probably aim to update to code whenever we have time and drop support for Webpack < 4.44.0 so we can simplify the code and use it as intended in Webpack 5.

Also, if you have time, can you confirm that it rewrites all generated d.ts for each rebuild when using watch mode even though the corresponding .ts file was not changed? I saw that behavior but haven't had the time to verify.

[alexander-akait]

I will review the PR and say how we can better keep compatibility with webpack@4 and webpack@5

[appzuka]

@JonWallsten, I believe this issue is caused by calling the function that emits the assets in the wrong hook. The function that emits the assets is called from makeAfterCompile which ultimately calls getEmitOutput. In there the emit function is used on the compiler object:

ts-loader/src/instances.ts

Line 819 in 268bc69

program.emit(

If I understand correctly, after webpack@4.40 we should be using emitAsset instead of emit. It is not clear to me what the benefit to emitAsset is, other than I assume at some point emit may be deprecated. I'm reluctant to just change .emit to .emitAsset because I don't understand the consequences. It would cause a breaking change, perhaps without any benefit. Even if we use emitAsset we still have the issue of which hook to use to call from, so I see these as separate issues.

We can fix this immediate issue without diving so deep and without causing a breaking change by implementing the solution you got working with help from @alexander-akait. The solution is compatible with webpack@4 and webpack@5. I'm planning to submit a PR implementing the code you provided so we can fix this bug. If anyone with a deeper understanding of ts-loader and webpack wants to propose a better solution that is great.

[JonWallsten]

@appzuka: Ah, I see. To be honest I don't have a good overview of exactly how this works since I had a single goal with my debugging rather then get a complete understanding.
I guess the idea was to simplify things, and correct me if I'm wrong on this, but I assumed that using the new way of adding assets might make things easier. Maybe @alexander-akait can fill us in on this one. If the only change is program.emit > compilation.emitAsset and there's no performance gain nor any code reduction it's obviously not worth the time until the old way is deprecated.

I thought this was the adding of the assets:

const assetPath = path.relative(compilation.compiler.outputPath,outputFile.name);
compilation.assets[assetPath] = {
    source: () => outputFile.text,
    size: () => outputFile.text.length,
};

So I clearly don't have a good enough understanding of this.

Anyway, yes, we should definitely do this in steps. First fix the issue, then maybe improve the code if we can.
One idea that instantly comes to mind is to put the subscription to the compilation hook in a better place so that it's not to late to get the first compilation instance.

Terser for example puts it in the apply method in the plugin's entry file.
https://github.com/webpack-contrib/terser-webpack-plugin/blob/master/src/index.js#L623

[appzuka]

@JonWallsten, I tried adding the hook at the start of the loader entry but it was already too late. Terser is a plugin, whereas ts-loader is a loader. It seems that for loaders the compilation is already created by the time the loader is called.

I can confirm that it re-creates the d.ts file each time the .ts source file is changed, even though the contents of the d.ts file may not have changed. However, it only re-creates d.ts files where the source .ts has changed. I created an example with a file sub.ts that was included by index.ts. sub.d.ts was created on the first run but not if just index.ts were updated.

On this point, when I initially tested the new code it recreated all files on every file change. This was because makeAfterCompile is a closure which closes over the variable checkAllFilesForErrors. If you call makeAfterCompile inside the hook you create a new closure for each hook and checkAllFilesForErrors is always true.

ts-loader/src/after-compile.ts

Line 37 in 6816735

let checkAllFilesForErrors = true;

I believe the code below works. I will release a PR with this code:

      // makeAfterCompile is a closure.  It returns a function which closes over the variable checkAllFilesForErrors
      // We need to get the function once and then reuse it, otherwise it will be recreated each time
      // and all files will always be checked.
      const cachedMakeAfterCompile = makeAfterCompile(
        instance,
        true,
        false,
        instance.configFilePath
      );

      // compilation is actually of type webpack.compilation.Compilation, but afterProcessAssets
      // only exists in webpack5 and at the time of writing ts-loader is built using webpack4
      const makeAssetsCallback = (compilation : any) => {
        compilation.hooks.afterProcessAssets.tap(
          'ts-loader',
          () => cachedMakeAfterCompile(compilation, () => {
            return null;
          })  
        );
      }

      // We need to add the hook above for each run.
      // For the first run, we just need to add the hook to loader._compilation
      makeAssetsCallback(loader._compilation);

      // For future calls in watch mode we need to watch for a new compilation and add the hook
      loader._compiler.hooks.compilation.tap('ts-loader', makeAssetsCallback);

I use the afterProcessAssets hook, not the processAssets with a stage. This is because ts-loader is currently built using webpack@4 and the stage constants do not exist. When a webpack5 only version of ts-loader is built this can be changed.

[carlosdp]

I seem to be running into this with ts-loader 9.2.6 and webpack 5.66.0

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

Closing as stale. Please reopen if you'd like to work on this further.