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

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

Open
minecrawler opened this issue Mar 23, 2023 · 22 comments
Open

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

minecrawler opened this issue Mar 23, 2023 · 22 comments

Comments

@minecrawler
Copy link

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
Copy link
Member

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

@minecrawler
Copy link
Author

minecrawler commented Mar 23, 2023

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
Copy link
Member

johnnyreilly commented Mar 23, 2023

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
Copy link
Author

minecrawler commented Mar 23, 2023

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
Copy link
Member

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
Copy link
Author

minecrawler commented Mar 24, 2023

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
Copy link
Member

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
Copy link
Author

minecrawler commented Mar 24, 2023

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
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Contributor

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
Copy link
Member

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

@minecrawler
Copy link
Author

minecrawler commented Mar 24, 2023

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
Copy link

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: cypress-io/cypress#26148

@johnnyreilly
Copy link
Member

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
Copy link
Contributor

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
Copy link

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

@andrewbranch
Copy link
Contributor

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
Copy link

lmiller1990 commented Mar 31, 2023

Great, thanks for this - will this patch in TypeScript mean ts-node, and ts-loader (and people who depend on those, like Cypress: cypress-io/cypress#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
Copy link
Contributor

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
Copy link
Member

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
Copy link

Looks like it's working in Cypress: cypress-io/cypress#26148 (comment)

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

4 participants