TypeStrong / ts-loader

TypeScript loader for webpack
https://johnnyreilly.com/ts-loader-goes-webpack-5
MIT License
3.44k stars 429 forks source link

ts-loader doesn't work when ts-config noEmit is set to true #1602

Open minecrawler opened 1 year ago

minecrawler commented 1 year ago

TypeScript: v5.0.2 Webpack: v5.76.3

Expected Behaviour

When bundling a ESM module with noEmit, Webpack is able to generate a bundle using ts-loader

Actual Behaviour

Module build failed (from ./node_modules/ts-loader/index.js):
Error: TypeScript emitted no output for C:\Users\ms\Projects\ts-loader\index.ts.
    at makeSourceMapAndFinish (C:\Users\ms\Projects\ts-loader\node_modules\ts-loader\dist\index.js:52:18)
    at successLoader (C:\Users\ms\Projects\ts-loader\node_modules\ts-loader\dist\index.js:39:5)
    at Object.loader (C:\Users\ms\Projects\ts-loader\node_modules\ts-loader\dist\index.js:22:5)

Steps to Reproduce the Problem

  1. Clone the demo repository
  2. npm-install
  3. npm-run-build it

Location of a Minimal Repository that Demonstrates the Issue.

https://github.com/minecrawler/ts-loader

Further info

connected to #161

johnnyreilly commented 1 year ago

What's the value of using ts-loader with noEmit? If ts-loader isn't emitting anything - what's it doing?

minecrawler commented 1 year ago

First of all, only tsc won't emit (it will transpile, but not emit). ts-loader, though, should pass transpiled code to Webpack, which should then create and emit the artifacts (e.g. a bundle)

The value is creating ESMs and compatibility with other runtimes (Deno). It leaves emitting to the bundler. The TypeScript devs reject emitting JS with modified imports, so they don't emit it and leave it to the bundler to resolve the actual imports. For Webpack, as the bundler, to support this, ts-loader must be able to work in this mode.

If you remove the noEmit, you will see this error:

image

Other tools, like esbuild, already support this!

johnnyreilly commented 1 year ago

The error message you are seeing comes from TypeScript / tsc; not from ts-loader. It looks like you might need to adjust your configuration.

minecrawler commented 1 year ago

yes, I changed the config intentionally to emit to show you that there's no way around noEmit. When I change my config back to satisfy tsc, it won't emit and I get the OG error... which as far as I can tell comes from ts-loader. If that's not the case, maybe you can point me in the correct direction.

As I said, other bundlers work fine with this config, so the problem seems to be either ts-loader or Webpack. There is no way around emit-less TypeScript for ESM, ts-extensions and other goodies, unfortunately...

johnnyreilly commented 1 year ago

The error message is:

TypeScript emitted no output

Your desire is to have noEmit - so it's behaving correctly. But with noEmit on, there is no output from TypeScript; it just type checks. No emit means no running program.

I suspect you want something different to noEmit; you want some kind of running program?

minecrawler commented 1 year ago

Your desire is to have noEmit

No, my desire is to use ts-extensions for my imports and using ESMs. For these options, there is, as the second error says, only two possibilities. Either noEmit, or emitDeclarationOnly. It's not possible to use the tsconfig in my sample repository without one of those. And I want the options I selected, because I'm developing libraries and applications which should support ESMs and Deno (next to NodeJS).

The message from the TypeScript team is very clear: the bundler must do the heavy-lifting, tsc won't emit (see the typescript PR). So, trying to use Webpack, I want a running JS program to be emitted. How can I do that? The default is to use ts-loader, but right now ts-loader cannot handle noEmit.

Please take a look at the example repository. I added working examples for the use-case with a second file (ts-extension), Deno, NodeJS, esbuild and ts-node. The resulting mjs can even be used in the browser. That's what I want :) Now it should just also work in Webpack, so I can migrate some of my larger projects to the new options.

I played around with tsc and it seems to emit some code without noEmit, but then runs into an error at some point. So I suspect ts-loader would have to call tsc without noEmit, and then handle the error. I'm not sure about that, though, since I don't know how it's done in other tools and working around an error sounds hacky.

johnnyreilly commented 1 year ago

I'm pretty sure (though I could be wrong) that noEmit, or emitDeclarationOnly are not what you're after. I'm also not sure that this this is the intent for how moduleResolution bundler was intended to be used. But again, I stand to be corrected. @andrewbranch can share any insight as to whether moduleResolution bundler was intended to unlock the described behaviour here? And if so, whether noEmit is intended to be in the mix? It seems surprising to me - but you never know

minecrawler commented 1 year ago

I'm pretty sure (though I could be wrong) that noEmit, or emitDeclarationOnly are not what you're after. I'm also not sure that this this is the intent for how moduleResolution bundler was intended to be used. But again, I stand to be corrected. @andrewbranch can share any insight as to whether moduleResolution bundler was intended to unlock the described behaviour here? And if so, whether noEmit is intended to be in the mix? It seems surprising to me - but you never know

It says so in the error above, and in the PR under "New compiler options":

allowImportingTsExtensions: Allow imports to include TypeScript file extensions. Requires '--moduleResolution bundler' and either '--noEmit' or '--emitDeclarationOnly' to be set.

andrewbranch commented 1 year ago

ts-loader is the only bundler scenario I know of that relies on tsc’s emit. For example, it’s conventional when using esbuild to have noEmit in your tsconfig. I knew this would be a problem for ts-loader and figured we’d need to cross that bridge eventually—or let it be a reason to encourage people to switch to transpileOnly mode.

minecrawler commented 1 year ago

@andrewbranch does that mean we'd need a multi-staged bundling workflow, where we first type-check with noEmit and then emit for ts-loader via transpileOnly?

andrewbranch commented 1 year ago

Yeah, something like that—that’s what https://github.com/TypeStrong/ts-loader#faster-builds describes via fork-ts-checker-webpack-plugin, but my sense is a lot of people are moving toward just relying on errors in-editor and in CI via a separate tsc step. It doesn’t make a ton of sense to make emit wait on type checking, in my opinion.

johnnyreilly commented 1 year ago

For what it's worth, I haven't used ts-loader with type checking on for about four years.

minecrawler commented 1 year ago

It doesn’t make a ton of sense to make emit wait on type checking, in my opinion.

I supervise several big projects, and we do type-checking and emitting in one step in our CI, currently. After all, we don't need artifacts if there are type errors 😆 It's just another way to think about it.

The pipelines are nothing I couldn't change, but I had the hope that we could just upgrade some versions and start using cool new features.

Well, if that's the way, then I at least hope for some documentation and maybe even a good error message, which tells me that I enabled noEmit with bundler/tsExtension and it's not supported in one step.

Also, do you have any best practices or experience you can share about how to setup Webpack for this kind of multi-step bundling? I'm not sure if just using two configs is the solution. Adding the best-practice to the docs as well would, of course, be wonderful!

lmiller1990 commented 1 year ago

ts-loader is the only bundler scenario I know of that relies on tsc’s emit

Just to clarify - if this is the case, ts-loader is going to be incompatible with allowImportingTsExtensions which requires noEmit: true?

I'm trying to get Cypress to work with ts-loader and the new alllowImportingTsExtensions, and running into a generic "Failed to compile" error. Issue: https://github.com/cypress-io/cypress/issues/26148

johnnyreilly commented 1 year ago

Without being an expert on allowImportingTsExtensions - it's possible that ts-loader may be incompatible. I'm assuming this wouldn't be straightforwardly supported given the way ts-loader works - at least without entering into hack territory. @andrewbranch do you have any thoughts?

andrewbranch commented 1 year ago

It’s my understanding that ts-loader is not compatible with allowImportingTsExtensions in full checking mode, but I thought it would work in transpileOnly: true, since I believe that uses the ts.transpileModule API where noEmit would be irrelevant. I could be wrong about the details there; it’s been a long, long time since I’ve looked at ts-loader’s source. But I think transpileOnly could be made to work if it doesn’t already.

lmiller1990 commented 1 year ago

Hm, I don't know much about how transpileOnly works, but I found a similar issue when using ts-node: https://github.com/TypeStrong/ts-node/issues/1981. I suppose these are actually the same issue at the core?

andrewbranch commented 1 year ago

I think you’re right, and the root cause is I was wrong about what “level” noEmit gets ignored at in transpileModule. It turns out any setting for noEmit gets wiped out right away rather than being ignored later in the process, and so the options validation happens the same way as it does in plain tsc. We’ll need to do something to suppress the error.

lmiller1990 commented 1 year ago

Great, thanks for this - will this patch in TypeScript mean ts-node, and ts-loader (and people who depend on those, like Cypress: https://github.com/cypress-io/cypress/issues/26148) will just work as expected when the patch is released? Do we need any work here? I can spend time on either code base, but it looks like #53599 should be all we need.

andrewbranch commented 1 year ago

I think it should just work. Note that in the meantime you may be able to just not pass allowImportingTsExtensions when transpiling. Though if you’re using the same config for a full check in another process, you might be stuck.

johnnyreilly commented 1 year ago

Hopefully you should be able to use something like:

https://github.com/TypeStrong/fork-ts-checker-webpack-plugin

And keep type checking - just keep it outside of ts-loader

lmiller1990 commented 1 year ago

Looks like it's working in Cypress: https://github.com/cypress-io/cypress/issues/26148#issuecomment-1493467682