Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

Using HappyPack with Awesome Typescript Loader #33

Closed
fenech opened this issue Apr 19, 2016 · 56 comments
Closed

Using HappyPack with Awesome Typescript Loader #33

fenech opened this issue Apr 19, 2016 · 56 comments

Comments

@fenech
Copy link

fenech commented Apr 19, 2016

This seems to be closely related to #32 but I didn't want to hijack that post by talking about a different TypeScript loader.

If possible, I would like to use HappyPack in the following way:

module: {
    loaders: [
        { test: /\.tsx?$/, loader: 'happypack/loader' }
    ]
},
plugins: [
    new HappyPack({
        loaders: [ 'awesome-typescript-loader' ]
    })
]

This is currently leading to errors like:

ERROR in ./App/Initialise.ts
Module parse failed: C:\path\to\src\node_modules\happypack\loader.js!C:\path\to\src\App\Initialise.ts Line 1: Unexpected identifier
You may need an appropriate loader to handle this file type.
| [object Promise]

Is it possible to use these two loaders together at the moment?

@amireh
Copy link
Owner

amireh commented Apr 20, 2016

I've tried this out quickly last night and was able to get it partially working with a few adjustments to both HappyPack's source and the loader's source.

The problem is that it also relies on webpack's compilers (like ts-loader) to add an after-compile plugin, we can't support that right now. However, in this loader, it seems that the plugin is only for emitting build errors / warnings which I believe should be achievable in a different approach.

I'll share the example I have locally soon.

@JamesHenry
Copy link

JamesHenry commented May 6, 2016

Look forward to it @amireh! :) HappyPack looks like such a cool idea, but I use TS on all my webpack projects, so haven't had chance to try it out yet.

@DanielRosenwasser
Copy link

DanielRosenwasser commented May 24, 2016

Hey there @amireh is there any way I can help out to unblock this?

@amireh
Copy link
Owner

amireh commented May 24, 2016

So, I've already revisited this and #40 a couple of times now, but there really is no way around it for loaders that hook into webpack's compiler or compilation. The only way I can see to be able to use those TS loaders is to make them stop relying on those webpack internals - write another loader, a simpler one or one that takes a different approach, or tune the existing one.

I mean, if there is specific functionality/APIs that are needed to make those loaders work, maybe we can support those, but doing things like compilation.plugin(...) is simply not possible. I just don't why the loaders are so tightly coupled to webpack's internals. Even webpack docs say it's "hacky access to the current compilation"...

The chunk I'm referring to is this one: https://github.com/s-panferov/awesome-typescript-loader/blob/master/src/instance.ts#L335-#L403 - if there's a way you can make that loader emit errors or do the work it's doing in that function without hooking into the "after-compile" phase, then we can make it work.

The APIs I see it's using are:

  • compilation.compiler.isChild() - can fake
  • compilation.bail - can fake
  • compilation.errors.push - can fake but not synchronously. I don't see why they don't use the emitError loader API that's already supported.
  • compilation.assets[] - can fake but not synchronously, maybe make an API "addAsset" that is available when the loader is not of type webpack (HappyPack turns this.webpack === false in the loader's context so we can differentiate)
  • compilation.fileDependencies.push - it should use the already existing api, this.addDependency

I'd be happy to help with something specific, but seeing as I don't use TypeScript myself it's hard for me to invest the time.

@DanielRosenwasser
Copy link

@amireh thanks for some of the guidance on that, especially since I'm not familiar with the internals. I think @jbrantly and @s-panferov would be able to better determine whether these changes would be feasible for their projects. Could you two weigh in a bit?

@jbrantly
Copy link

Speaking for ts-loader, the reason it's so tightly coupled to webpack is because webpack's loader API uses a pretty simple convention of input => output but for TypeScript it's not so simple, mostly due to dependencies. For example, hooking into after-compile is done because it's more efficient to report errors from the program as a whole instead of from each individual file. More importantly, it's necessary to be able to report errors from files that webpack never sees like definition files. That's also the reason for using things like compilation.errors.push, because you can't use emitError from a loader invocation that never happens (because it's a file that webpack never sees).

One thing to try would be ts-loader's transpileOnly option which much more closely mimics webpack's input => output paradigm and skips a number of the webpack integrations (but not all) at the cost of reduced error reporting. I believe awesome-typescript-loader has a similar option but I'm not sure how it affects webpack integrations.

@amireh
Copy link
Owner

amireh commented May 30, 2016

@jbrantly Thanks for explaining, that makes sense. So what I'm thinking is that we could have ts-loader check if happypack is running (we'll expose a this.happy === true on the loader context) and use custom APIs for doing the exact same thing that it would do in the after-compile phase, only in a manner that HappyPack can serialize between processes.

So, for example, instead of compilation.errors.push it would be compilation.pushError() so that we can do it asynchronously (and ts-loader wouldn't really care since there's no return value.)

The way I'd do this is by hooking into after-compile in the main process and then orchestrate the loaders' operations from there.

If that's too weird, there's another option of allowing a function to be passed up to a compiler but with frozen arguments (and the function can't rely on any closure-bound variables since it won't be executed in that scope.) Then ts-loader would prepare the variables it needs ahead of time, pass the function body (toString) and those parameters to HappyPack, and then we execute it on the main thread.

I prefer the first approach because it's more flexible, less bizarre, and allows us to introduce APIs that can yield values in the future if needed.

Thoughts?

@IAMtheIAM
Copy link

IAMtheIAM commented May 31, 2016

+1 for wanting to get HappyPack to work with a typescript loader.

@IAMtheIAM
Copy link

How likely is typescript support going to be added, and how long will it take? I will help develop if I knew what to do.

@TheLarkInn
Copy link

TheLarkInn commented Jun 8, 2016

@amireh @fenech have you tried setting useWebpackText: true this tells a-t-loader to use webpack's method of serving files to the loader. It is an awesomeTypescriptLoaderOptions Config option.

@TheLarkInn
Copy link

@DanielRosenwasser #33 (comment)

That is why the ts-loader's use LanguageServiceHost

@IAMtheIAM
Copy link

@TheLarkInn I tried setting useWebpackText: true in tsconfig.json and using happypack, but still got "you may need an appropriate loader"

@amireh
Copy link
Owner

amireh commented Jun 16, 2016

I'll be taking a dive at this over the next weekend. I'll be trying out the suggestions made in my latest comment above so that we don't lose out on any current loader functionality (e.g. program-level error reporting.)

@amireh
Copy link
Owner

amireh commented Jun 20, 2016

As an update, I got the initial build to function. There were some artifacts but nothing breaking. However, the --watch mode is a different story and I'm not sure if we'll be able to support that...

@IAMtheIAM
Copy link

IAMtheIAM commented Jun 21, 2016

Thanks for an update. Will your initial build that you got working work properly with Webpack-Dev-Server using hot module replacement (instead of webpack --watch)

@amireh
Copy link
Owner

amireh commented Jun 23, 2016

I actually didn't try the devserver as I don't use it normally. I'll give it a go (but I suspect it still uses webpack's watcher internally and if it does then it's the same thing)

Edit: it might also be a good idea to push this onto a WIP branch / PR that you guys can test because really, I'm just testing fake TS sources and we need to test on a real repository with a good number of modules.

@TheLarkInn
Copy link

But it builds? That's still a very good step in the right direction!!!!!

@IAMtheIAM
Copy link

Feel free to put in a WIP branch that we can test, I will play with it and see what happens.

@amireh
Copy link
Owner

amireh commented Jun 28, 2016

I did, see #56 . So I started with awesome-typescript-loader, then switched to ts-loader because there's less going on there, I'd recommend that u first test that one.

Let me know on the PR if you can/can't get it to work. There are happypack-support branches in my forks of the loader repos that you need to pull down.. it's ugly and takes some work, if you have a better idea how to test this I'm all ears.

@IAMtheIAM
Copy link

IAMtheIAM commented Aug 5, 2016

@amireh I uploaded my project source so you can test out TypeScript compilation on a working Angular2-TypeScript-Webpack application. See pull request #56 comment here

I'm still hoping we can figure out a way to get HappyPack to work with typescript, because honestly, after using TypeScript, I am unlikely to go back to vanilla JS since TypeScript offers many benefits. I'm sure most TypeScript devs feel similar also.

@amireh
Copy link
Owner

amireh commented Aug 5, 2016

@IAMtheIAM I did see that but I still didn't find the time to get into, my apologies. It's just much more involved than other tickets around here and I hardly find more than an hour or so to put into this.

I appreciate your patience.

@deevus
Copy link

deevus commented Aug 31, 2016

If like me you're piping ts-loader into babel-loader then react-hot I sped up my compilation by doing the following:

    plugins: [
      new HotModuleReplacementPlugin({multiStep: true}),
      new HappyPack({
        loaders: ["react-hot", "babel"]
      })
    ],
    module: {
      loaders: [
        {
          exclude: join(__dirname, "..", "..", "node_modules"),
          loaders: ["happypack/loader", "ts"],
          test: /\.tsx?$/
        }
      ]
    }

So, if you're using ts-loader or awesome-typescript-loader to do everything, you could defer some parts (like 6to5 and jsx transformation) to babel-loader which will allow you to use happypack in some capacity

@IAMtheIAM
Copy link

IAMtheIAM commented Sep 16, 2016

@deevus Could you explain that a little more? I have been using awesome-typescript-loader but I haven't been piping it into anything. How would we go about your suggested changes if awesome-typescript-loader was doing all the work previously?

@deevus
Copy link

deevus commented Sep 17, 2016

@IAMtheIAM Personally I'm using ts-loader but as long as tsconfig.json works it should function the same. I have target set to es6 and jsx set to preserve. So ts-loader outputs es6 with jsx in tact, which is then piped into babel-loader which compiles to es5 for browser consumption. react-hot is included for development.

@jacob-israel-turner
Copy link

Any progress on this lately? Any way I can help out?

@aindlq
Copy link

aindlq commented Mar 2, 2017

@abergs Yes I'm still using the old version of ts-loader that I patched for happypack, so far so good, see https://github.com/aindlq/ts-loader/tree/happypack

So far I didn't find the time to try the latest ts-loader.

@johnnyreilly
Copy link
Contributor

johnnyreilly commented May 20, 2017

@aindlq would you be interested in submitting a PR to ts-loader around the changes you suggest? I'd be interested in unblocking this for transpileOnly mode.

TypeStrong/ts-loader#336

@johnnyreilly
Copy link
Contributor

Fwiw I know the codebase has moved on since your initial work but if you're able to share what those lines are I should be able to help identify the latest codebase equivalent

@johnnyreilly
Copy link
Contributor

Finally do you have an example setup of using ts-loader with happypack that you'd be willing to share for reference?

@johnnyreilly
Copy link
Contributor

Ps I just got interested in happypack... Can you tell ;-)

@aindlq
Copy link

aindlq commented May 20, 2017

@johnnyreilly my old change is here - https://github.com/aindlq/ts-loader/commit/8d5badbdbb9e13838e321405b68c252fc484f9c9#diff-ed009b6b86b017532ef0489c77de5100, from what I can see it is almost the same in the new code base.

I've just migrated my project to latest webpack and ts-loader and trying to use https://github.com/webpack-contrib/cache-loader instead of happypack. I'll try to apply my patch tothe latest happypack and compare it with cache-loader, will let you know the result.

@aindlq
Copy link

aindlq commented May 20, 2017

@johnnyreilly OK, it actually works, but I'm not sure if my changes are actually correct.

351 ts file:
baseline - 30 seconds
happypack with ts-loader - 24 seconds cold, 22 seconds hot
cache-loader with ts-loader - 32 seconds cold, 22 seconds hot

As you can see happypack can significantly improve initial compile time. But afterwards it is equivalent to cache-loader. As for the issues in ts-loader, they are the same as described here - TypeStrong/ts-loader#336

Basically loader._module.errors is undefined here - https://github.com/TypeStrong/ts-loader/blob/master/src/instances.ts#L68 and this._module.meta is undefined here - https://github.com/TypeStrong/ts-loader/blob/master/src/index.ts#L49. I just comment these lines and somehow it works, but probably it is not the right way to do it.

@amireh
Copy link
Owner

amireh commented May 20, 2017

@aindlq thanks for mentioning cache-loader. In truth, I am planning on removing the caching functionality from happypack altogether because it's widening the concern of the library and it wasn't my original intent. In order to do that, however, something had to be introduced in its place to maintain the functionality for those who use it. Now that you've pointed that loader out, you saved me the work! 😁

On another front, the benchmarks you show lead me to assume that you're still capped by the single TS server that's doing your compilations because normally I'd have anticipated a bigger change from introducing happypack to the build.

Is it possible to have multiple typescript servers running at the same time and instruct happypack to switch between them? (I bet you can utilize a load balancer like haproxy to do this without touching any code at all, in fact.)

@johnnyreilly
Copy link
Contributor

johnnyreilly commented May 21, 2017

@aindlq - that's super helpful; thank you! What I have in mind is potentially adding an extra option to ts-loader. Something like happyPackMode. If ts-loader was started with that option then transpileOnly would be implicitly set to true.

Based on what you've said, we'd need to make the following (minimal) changes to ts-loader:

https://github.com/TypeStrong/ts-loader/blob/master/src/instances.ts#L68 would go from this:

            loader._module.errors,

to this:

            loaderOptions.happyPackMode ? undefined : loader._module.errors,

and https://github.com/TypeStrong/ts-loader/blob/master/src/index.ts#L49 would go from this:

    this._module.meta.tsLoaderFileVersion = fileVersion;

to this:

    if (!options.useHappyPack) {
        this._module.meta.tsLoaderFileVersion = fileVersion;
    }

And obviously useHappyPack (or whatever the flag ends up getting called) would need to be added to the LoaderOptions interface and defaulted to false by default here: https://github.com/TypeStrong/ts-loader/blob/master/src/index.ts#L74

Would you be interested in creating a PR for ts-loader that introduces this? Given the nature of the minimal changes needed to support happypack behind a flag I'm of the opinion that it seems entirely reasonable to add it.

cc @jbrantly

@johnnyreilly
Copy link
Contributor

johnnyreilly commented May 21, 2017

@amireh on your question here:

Is it possible to have multiple typescript servers running at the same time and instruct happypack to switch between them?

I'm not certain myself but I have a feeling this may be possible. @piotr-oles may be able to advise.

On a wider note the reason that I've suddenly got interested in happypack is because, having previously had no real use for transpileOnly mode myself I now do. That's because of @piotr-oles work on: https://github.com/Realytics/fork-ts-checker-webpack-plugin which allows performing type checking outside of ts-loader entirely. @aindlq you might find this plugin useful based on what you've said.

If you look at the docs for the plugin you'll note the workers section here:

workers number - You can split type checking to few workers to speed-up on increment build. Be careful - if you don't want to increase build time, you should keep 1 core for build and 1 core for system free (for example system with 4 cpu threads should use max 2 workers).

I haven't looked into the implementation of this at all but it's possible it works by having multiple typescript servers. Hopefully @piotr-oles can say something.

@aindlq
Copy link

aindlq commented May 21, 2017

@piotr-oles
Copy link

@amireh, @johnnyreilly

I haven't looked into the implementation of this at all but it's possible it works by having multiple typescript servers. Hopefully @piotr-oles can say something.

In fork-ts-checker-webpack-plugin I use compiler API, not server, but it's possible to run many ts servers. They use standard i/o, not TCP, so you can spawn as many ts server processes as you need (they are not using same port or socket).

Having many tsservers/compilers have sense only with transpileOnly: true option. Using semantic checker on many workers leads to work duplication. It's because with type checking, a compiler has to analyze all imported files and then files imported by theses files and so on (to ensure semantic correctness). I've tried this on a big project and, paradoxically, it took a little bit longer to compile on 4 threads than on 1 (with semantic checker).

@johnnyreilly
Copy link
Contributor

Thanks @piotr-oles. BTW happypack seems to play well with your plugin based on the quick test I just did. I'd be interested to know if happypack speeds up your build times any further.

@amireh
Copy link
Owner

amireh commented May 21, 2017

They use standard i/o, not TCP, so you can spawn as many ts server processes as you need (they are not using same port or socket).

Then that should be doable at the ts-loader level I assume, where it would accept a number of servers to spawn and rotate between them for every file it's loading (or indeed, we can have happypack expose the current thread ID and ts-loader can in turn use that to map to a server index.)

I'd be happy to experiment with this if there's interest.

@johnnyreilly
Copy link
Contributor

Sounds interesting - if you fancy experimenting then go for it. We're open to making further changes to ts-loader to support happypack usecases

@johnnyreilly
Copy link
Contributor

BTW feel free to update your wiki to list ts-loader support for happypack 😄

@piotr-oles
Copy link

Good job guys, with happy pack my initial build time drops from 80 to 50 seconds :)

@abergs
Copy link

abergs commented May 22, 2017

I'm sharing my experince here as well!

Today (in about 30 minutes) we switched from awesome-typescript-loader (with the type-checker-fork) to happypack + ts-loader + fork-ts-checker-webpack-plugin.

We moved from 1min 32s (with babel cache + DLL) to 1 min 17 seconds (cold cache) and 56s warm cache.
image

Quite a win, but still surprised over the time spent building modules. Have I missed anything? Webpack counts about 2200 modules, however happypack cache gives us about 1213 in cache:
image

For those interestind in making the switch, I suggest looking at the commit in the pull request as a guidance (https://github.com/TypeStrong/ts-loader/pull/547/files)

I ran the build on a 2yr old dual core laptop.

@johnnyreilly
Copy link
Contributor

Thanks so much for sharing @abergs!

@sompylasar
Copy link

Following @abergs success, succeeded upgrading JS-only to JS+TS decent size project from one HappyPack instance for JS to two HappyPack instances, the JS one: babel-loader, eslint-loader with babel-eslint; the TS one: ts-loader, babel-loader, eslint-loader with typescript-eslint-parser; and fork-ts-checker-webpack-plugin for TS typechecking; all works with both browser-side and server-side hot reload.

@johnnyreilly
Copy link
Contributor

Brilliant - thanks so much for letting us know @sompylasar! I'll plan to put some examples of how to use ts-loader with happypack in the ts-loader repo when I get a moment.

@abergs
Copy link

abergs commented May 24, 2017

When switching from Awesome Typescript Loader to ts-loader we also had to add "sourceMaps: true" in our .tsconfig to get sourcemaps. Makes sense, but was unexpected

@johnnyreilly
Copy link
Contributor

Cool - I seem to recall that was a deliberate choice made by @jbrantly. Tbh I always have that set to true anyway!

@jbrantly
Copy link

Yea... I think I went back and forth a bit on that. The original idea was to be as close to tsc as possible to prevent the inverse issue ("it works in tsc but not in ts-loader").

@amireh
Copy link
Owner

amireh commented Jun 1, 2017

Can someone make a PR that includes an example of using ts-loader with happypack? It may also be a good idea to write a wiki article for this since there's some explanation to be provided.

Thoughts?

EDIT: I just noticed #164 which is related, oops! A PR would still be very much welcome since I'm out of context here.

@johnnyreilly
Copy link
Contributor

PR submitted!

@amireh
Copy link
Owner

amireh commented Jun 2, 2017

The README now indicates TypeScript compatibility - thank you all for your work.

I still have to fill out the wiki article with some instructions but with the example in place this is very good.

We can finally close this issue. ❤️

@amireh amireh closed this as completed Jun 2, 2017
@johnnyreilly
Copy link
Contributor

Brilliant - there's a minor change I should make to the example as the API of fork-ts-checker-webpack-plugin has slightly changed as part of @piotr-oles work. Basically just deleting one line. Will submit shortly - thanks so much!

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

No branches or pull requests