webpack 5 migration #1251

johnnyreilly · 2021-02-04T20:43:19Z

This PR will be where the migration of ts-loader to webpack 5 takes place. Help is appreciated!

dependencies upgraded to webpack 5 (enhanced-resolve, webpack-cli, webpack)
drop support for webpack 4
make node 12 the minimum version
code compiles
~~drop support for experimental watch API~~ - see webpack 5 migration #1251 (comment)
upgrade comparison test pack running against webpack 5
execution tests pass
increment major version to 9.0 and update CHANGELOG.md
update README.md

cc @alexander-akait @appzuka @JonWallsten - all advice and contributions towards this are gratefully received.

Feel free to PR or push into this branch to work on this.

Some discussion here: webpack/webpack#12475 (reply in thread)

UPDATE

Okay I think we're ready to ship. I'm going to:

create a branch for webpack 4 - so if there's a need to ship another change to support webpack 4 we can (I don't expect to but you never know)
write up a blog post to detail this
ship

appzuka · 2021-02-04T20:54:50Z

Would this be a good point to drop support for experimentalWatchApi? As far as I can see it has never worked and it adds complexity to the code.

JonWallsten · 2021-02-04T21:10:27Z

Don't forget to update webpack-cli to 4.5.0 (I think it's the latest).

johnnyreilly · 2021-02-04T21:45:32Z

Would this be a good point to drop support for experimentalWatchApi? As far as I can see it has never worked and it adds complexity to the code.

Go for it @appzuka - added to checklist above

johnnyreilly · 2021-02-04T21:46:48Z

Don't forget to update webpack-cli to 4.5.0 (I think it's the latest).

Good shout - added to checklist above

JonWallsten · 2021-02-05T09:00:53Z

@johnnyreilly How do you normally run the loader when you are working? Do you have an example project you are running locally where you build with a local copy of the loader?

In short: What's your most optimized workflow?

Edit: I cloned a simple Webpack 4 + Typescript repo, upgraded all deps and added a symlink to my local ts-loader clone. Should be enough I think.

JonWallsten · 2021-02-05T09:37:35Z

@johnnyreilly: How do you wanna handle these scenarios?

Potential fix:

johnnyreilly · 2021-02-05T09:39:52Z

That seems like a legitimate workflow @JonWallsten - to your question about the fix: yeah maybe. Question: is it legitimate that a module may be a NormalModule? If so then seems reasonable

JonWallsten · 2021-02-05T09:42:52Z

@johnnyreilly / @appzuka:
Getting back to this:

Am I correctly to assume that the first images shows where we ask TypeScript to compile and the second we pass those assets to Webpack?

Edit: So it seems like the ts-code is emitted in the first image and the d.ts and d.ts.map files are emitted in the second image. Makes sense since they should be "additional assets".

This seems to work fine:

JonWallsten · 2021-02-05T09:46:12Z

That seems like a legitimate workflow @JonWallsten - to your question about the fix: yeah maybe. Question: is it legitimate that a module may be a NormalModule? If so then seems reasonable

According to the debugger, the current module I'm looking at is a NormalModule.

And accroding to the types:

NormalModule is the only Module related class that has the property resources. So we can assume that it has to be a NormalModule? I don't see any other way of getting the types to work besides setting it as any. And that would be sad! :(

johnnyreilly · 2021-02-05T09:57:57Z

If that's what the debugger says, I trust it!

JonWallsten · 2021-02-05T10:03:23Z

If that's what the debugger says, I trust it!

I can't promise that all modules are NormalModules, but if we trust the types, the resource prop only exists on NormalModule and since we do a nullcheck on that property in extension we should be able to check if it's a NormalModule so we get the correct Type information in the editor.

appzuka · 2021-02-05T11:07:35Z

@JonWallsten,

I put some logging statements to clarify where assets are transpiled and emitted. When running on a small project (index.ts with sub.ts as a dependency) I get the following output:

Processing /home/nick/scratch/ts-loader-1243/src/index.ts
getEmitOutput:Sourcefile: /home/nick/scratch/ts-loader-1243/src/index.ts
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/index.js
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/index.d.ts.map
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/index.d.ts
Returning transpiled file to webpack: /home/nick/scratch/ts-loader-1243/src/index.ts

Processing /home/nick/scratch/ts-loader-1243/src/sub.ts
getEmitOutput:Sourcefile: /home/nick/scratch/ts-loader-1243/src/sub.ts
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/sub.js
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/sub.d.ts.map
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/sub.d.ts
Returning transpiled file to webpack: /home/nick/scratch/ts-loader-1243/src/sub.ts

** Running processAssets hook
getEmitOutput:Sourcefile: /home/nick/scratch/ts-loader-1243/src/index.ts
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/index.js
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/index.d.ts.map
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/index.d.ts
>> outputFileToAsset: index.d.ts.map
>> outputFileToAsset: index.d.ts
getEmitOutput:Sourcefile: /home/nick/scratch/ts-loader-1243/src/sub.ts
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/sub.js
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/sub.d.ts.map
getEmitOutput:Output: /home/nick/scratch/ts-loader-1243/dist/sub.d.ts
>> outputFileToAsset: sub.d.ts.map
>> outputFileToAsset: sub.d.ts

I believe the program.emit statement in getEmitOutput is where the output from the TypeScript compiler is obtained. The input is a single .ts file and the output is a .js file and .d.ts and .d.ts.map files, if required.

The .js file is returned to webpack as the result of the compilation. This happens until all .ts files have been processed.

Then, during the processAssets hook, makeAfterCompile goes through all the files the TypeScript instance viewed, gets the outputs files from program.emit again, and adds .d.ts and .d.ts.map files as assets.

It seems inefficient that program.emit is called multiple times to get the output from .ts files, but I believe the output is cached so on subsequent calls it is just hitting the cache.

On this small example, each asset is only emitted to webpack once. So it seems that the asset output caching recommended by @alexander-akait may not be required. However, when I run this in watch mode the assets are emitted even if their contents have not changed, so there could be a benefit in implementing a cache around the asset output in watch mode. There could also be a benefit in more complex projects.

JonWallsten · 2021-02-05T12:19:07Z

@appzuka: Thanks for the explanation! Do we know why the files are emitted in different steps? Is it supposed to be like that or just legacy?

johnnyreilly · 2021-02-06T10:37:10Z

Hey @sokra

I hope you are keeping well!

I seem to remember you put together a doc which detailed the API changes between webpack 4 and 5 targeted at maintainers of loaders and plugins. We're finally getting round to building ts-loader with webpack 5 and bumping on a few missing pieces.

Do you happen to remember where that doc lives? For example we're trying to track down webpack.loader.LoaderContext and not finding it.

All help gratefully received!

appzuka · 2021-02-06T11:08:53Z

@JonWallsten

Do we know why the files are emitted in different steps? Is it supposed to be like that or just legacy?

I understand why the js is returned separately from the .d.ts files but not why they are returned at different stages. A simple loader as described at https://webpack.js.org/contribute/writing-a-loader/ just takes an input file and transforms it to a string that is valid Javascript. In the case of ts-loader the input file is a .ts file, which is transformed into Javascript but also .d.ts and other files need to be added to the assets in the output directory. The Javascript output by typescript is not emitted as an asset but rather passed back to webpack for inclusion in the bundle.

What is not clear to me is why the .d.ts and other files are emitted together when all the .ts files have been processed rather than being emitted as assets during the getEmitOutput. It would make things simpler as we would not need the compilation/processAssets hooks at all, but perhaps there is a reason it was done this way.

It seems to me that processAssets is not really the right place to generate the assets. In the Terser example processAssets was used but then Terser is a plugin which processes assets generated by other plugins and loaders. In ts-loader we are not processing the assets, we are creating them.

I'm wary of changing the architecture in case we are repeating a mistake someone fixed several years ago, although simplifying ts-loader may also help improve performance and reduce errors.

johnnyreilly · 2021-02-06T12:55:02Z

I'm wary of changing the architecture in case we are repeating a mistake someone fixed several years ago, although simplifying ts-loader may also help improve performance and reduce errors.

I have a feeling that there was work done around this but it was a long time ago and frankly I don't remember the details. I'm open to changes being made but it's possible there's a reason things are the way they are now and I'm just not recalling

JonWallsten · 2021-02-06T16:06:10Z

It kind of make sense though that the d.ts and d.ts.map files are concidered additional files since they are not part of the bundle and Webpack is not built around TypeScript. So even though they are vital to us, it's not vital to Webpack in general. Or am i missing something?

johnnyreilly · 2021-02-18T05:08:47Z

It kind of make sense though that the d.ts and d.ts.map files are concidered additional files since they are not part of the bundle and Webpack is not built around TypeScript. So even though they are vital to us, it's not vital to Webpack in general. Or am i missing something?

Sorry I missed this @JonWallsten - I don't quite follow the question.

Incidentally, it turns out that webpack isn't exporting all types... So some we'll have to hold onto ourselves

… feat/webpack-5

johnnyreilly · 2021-02-18T06:04:17Z

Okay - the code compiles now; I borrowed a bunch of stuff from your branch @JonWallsten - thanks!

No tests pass alas

johnnyreilly · 2021-02-18T06:12:17Z

Execution tests passing! (or it looks like it!)

JonWallsten · 2021-02-18T06:44:10Z

It kind of make sense though that the d.ts and d.ts.map files are concidered additional files since they are not part of the bundle and Webpack is not built around TypeScript. So even though they are vital to us, it's not vital to Webpack in general. Or am i missing something?

Sorry I missed this @JonWallsten - I don't quite follow the question.

Incidentally, it turns out that webpack isn't exporting all types... So some we'll have to hold onto ourselves

This is like jeopardy. You give the answer I ask the question! 😂 Why is ts-files and d.ts/d.ts.map files emitted in different places in the code and Webpack lifecycle. I would say it's because declaration files are not part of the "code" in that sense, it's additional bonus stuff that will end up as separate files, not part of the bundle, in contrast to the transpiled/compiled (which is it?) code and the sourcemaps.

johnnyreilly · 2021-04-16T04:28:27Z

Did you have a chance to try it out @JonWallsten? All feedback welcome!

JonWallsten · 2021-04-16T06:00:19Z

Did you have a chance to try it out @JonWallsten? All feedback welcome!

Sorry! Got stuck with build issues in our project yesterday. I will try it out today instead.

JonWallsten · 2021-04-16T09:11:12Z

I have tried to build a prod build, dev builds with --watch and changed a few files. Seems to work fine. Not slower nor faster then previously. No errors.

JonWallsten · 2021-04-16T09:12:39Z

I guess we will get additional feedback once released! :)

appzuka · 2021-04-16T15:55:06Z

@johnnyreilly , @JonWallsten, I don't believe I have contributed to this branch but I built a couple of my projects with the latest feat/webpack-5 branch today. They worked fine and I did not notice any performance or size issues. Well done.

johnnyreilly · 2021-04-16T17:27:41Z

Thanks @appzuka (very pleased that you gave it a test!)

JonWallsten · 2021-04-16T19:09:42Z

src/instances.ts

-      // Consider changing this when ts-loader is built using webpack5
-    };
+  // It may be better to add assets at the processAssets stage (https://webpack.js.org/api/compilation-hooks/#processassets)
+  // This requires Compilation.PROCESS_ASSETS_STAGE_ADDITIONAL, which does not exist in webpack4


@johnnyreilly: I forgot about this. I think it was part of my PR when I sidetracked with the GitHub Workflow thing.

Let's fix that in a minor later!

into feat/webpack-5

johnnyreilly · 2021-04-16T20:43:22Z

Cool - I'll finish writing up the blog post and we'll ship ⛴️

JonWallsten · 2021-04-18T18:34:18Z

@johnnyreilly: Woho! Super good work @johnnyreilly! 🏆Sorry I couldn't be more helpful the last couple of weeks!

johnnyreilly · 2021-04-18T18:35:57Z

Thanks @JonWallsten - I appreciate the help that you gave!

johnnyreilly added 2 commits February 4, 2021 20:42

start webpack 5 migration

f710e4a

fixes

64d0362

johnnyreilly mentioned this pull request Feb 4, 2021

Update definition files in watch mode in webpack@5 #1249

Merged

johnnyreilly mentioned this pull request Feb 5, 2021

Repeated reads of same files using ts-loader vs tsc #1245

Closed

johnnyreilly added 2 commits February 18, 2021 05:41

Merge branch 'master' of https://github.com/TypeStrong/ts-loader into…

9c0053b

… feat/webpack-5

compiles now

ad9b5da

upgrade karma webpack

18fc051

drop webpack 4 code branches

f7dbab6

johnnyreilly changed the title ~~start webpack 5 migration~~ webpack 5 migration Feb 18, 2021

This was referenced Apr 16, 2021

Could we strongly type LoaderContext? webpack/webpack#13162

Closed

feat: add LoaderContext to types webpack/webpack#13164

Merged

update vanilla example

1df5557

JonWallsten reviewed Apr 16, 2021

View reviewed changes

johnnyreilly added 2 commits April 16, 2021 19:56

use webpack type

2edf96c

Merge branch 'feat/webpack-5' of https://github.com/TypeStrong/ts-loader

f3089d2

into feat/webpack-5

johnnyreilly added 2 commits April 17, 2021 09:16

enrich WebpackLoaderContext type

acbc71f

target es2018

62558c0

johnnyreilly merged commit c2d1ca2 into main Apr 18, 2021

johnnyreilly deleted the feat/webpack-5 branch April 18, 2021 18:25

mcdmaster mentioned this pull request Apr 21, 2021

Update dependency ts-loader to v9 Tokyo-Metro-Gov/covid19#6183

Closed

1 task

johnnyreilly mentioned this pull request Aug 5, 2021

Unable to load default and exported from vue templates #1351

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

webpack 5 migration #1251

webpack 5 migration #1251

johnnyreilly commented Feb 4, 2021 •

edited

appzuka commented Feb 4, 2021

JonWallsten commented Feb 4, 2021

johnnyreilly commented Feb 4, 2021 •

edited

johnnyreilly commented Feb 4, 2021 •

edited

JonWallsten commented Feb 5, 2021 •

edited

JonWallsten commented Feb 5, 2021

johnnyreilly commented Feb 5, 2021

JonWallsten commented Feb 5, 2021 •

edited

JonWallsten commented Feb 5, 2021

johnnyreilly commented Feb 5, 2021

JonWallsten commented Feb 5, 2021

appzuka commented Feb 5, 2021

JonWallsten commented Feb 5, 2021

johnnyreilly commented Feb 6, 2021

appzuka commented Feb 6, 2021

johnnyreilly commented Feb 6, 2021

JonWallsten commented Feb 6, 2021 •

edited

johnnyreilly commented Feb 18, 2021

johnnyreilly commented Feb 18, 2021

johnnyreilly commented Feb 18, 2021

JonWallsten commented Feb 18, 2021

johnnyreilly commented Apr 16, 2021

JonWallsten commented Apr 16, 2021

JonWallsten commented Apr 16, 2021 •

edited

JonWallsten commented Apr 16, 2021

appzuka commented Apr 16, 2021

johnnyreilly commented Apr 16, 2021 •

edited

JonWallsten Apr 16, 2021

JonWallsten Apr 17, 2021

johnnyreilly commented Apr 16, 2021

JonWallsten commented Apr 18, 2021

johnnyreilly commented Apr 18, 2021

webpack 5 migration #1251

webpack 5 migration #1251

Conversation

johnnyreilly commented Feb 4, 2021 • edited

UPDATE

appzuka commented Feb 4, 2021

JonWallsten commented Feb 4, 2021

johnnyreilly commented Feb 4, 2021 • edited

johnnyreilly commented Feb 4, 2021 • edited

JonWallsten commented Feb 5, 2021 • edited

JonWallsten commented Feb 5, 2021

johnnyreilly commented Feb 5, 2021

JonWallsten commented Feb 5, 2021 • edited

JonWallsten commented Feb 5, 2021

johnnyreilly commented Feb 5, 2021

JonWallsten commented Feb 5, 2021

appzuka commented Feb 5, 2021

JonWallsten commented Feb 5, 2021

johnnyreilly commented Feb 6, 2021

appzuka commented Feb 6, 2021

johnnyreilly commented Feb 6, 2021

JonWallsten commented Feb 6, 2021 • edited

johnnyreilly commented Feb 18, 2021

johnnyreilly commented Feb 18, 2021

johnnyreilly commented Feb 18, 2021

JonWallsten commented Feb 18, 2021

johnnyreilly commented Apr 16, 2021

JonWallsten commented Apr 16, 2021

JonWallsten commented Apr 16, 2021 • edited

JonWallsten commented Apr 16, 2021

appzuka commented Apr 16, 2021

johnnyreilly commented Apr 16, 2021 • edited

JonWallsten Apr 16, 2021

Choose a reason for hiding this comment

JonWallsten Apr 17, 2021

Choose a reason for hiding this comment

johnnyreilly commented Apr 16, 2021

JonWallsten commented Apr 18, 2021

johnnyreilly commented Apr 18, 2021

johnnyreilly commented Feb 4, 2021 •

edited

johnnyreilly commented Feb 4, 2021 •

edited

johnnyreilly commented Feb 4, 2021 •

edited

JonWallsten commented Feb 5, 2021 •

edited

JonWallsten commented Feb 5, 2021 •

edited

JonWallsten commented Feb 6, 2021 •

edited

JonWallsten commented Apr 16, 2021 •

edited

johnnyreilly commented Apr 16, 2021 •

edited