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 with Typescript 4.5 beta: "Error: TypeScript emitted no output for .../index.mts" #1383

Closed
JasonKleban opened this issue Oct 11, 2021 · 47 comments · Fixed by #1503
Closed

Comments

@JasonKleban
Copy link

JasonKleban commented Oct 11, 2021

Anyone have luck webpack-5-ing with the new .mts extension? I'm able to output from tsc -p . fine, but through webpack I'm getting "Error: TypeScript emitted no output for .../index.mts".

Thanks!

    resolve: {
      // Add `.ts` and `.tsx` as a resolvable extension.
      extensions: [".mts", ".ts", ".tsx", ".js"]
    },
    module: {
      rules: [
        {
            test: /(\.mts$)|(\.tsx?$)/,
            use: [
                {
                    loader: 'ts-loader',
                    options: {
                        //transpileOnly: true
                        compilerOptions: {
                            noEmit: false
                        }
                    }
                }
            ]
        }
      ]
    }
@johnnyreilly
Copy link
Member

Haven't tried this at all - am interested with how this pans out!

@JasonKleban
Copy link
Author

JasonKleban commented Oct 12, 2021

Yes, I believe it is because of the inclusion of the eventual extension.

import { foo } from "./misc" works as always
import { foo } from "./misc.ts" is wrong and gives TS2691: An import path cannot end with a '.ts' extension. Consider importing './misc.js' instead.

but

import { foo } from "./misc.js"; works with tsc but fails only in webpacking with Module not found: Error: Can't resolve './misc.js' in 'C:\example\src' resolve './misc.js'

Does ts-loader need to correct these paths for resolution? Or what?

@johnnyreilly
Copy link
Member

I'm not sure - webpack may need to be configured directly to support modules. Probably worth checking that out first. It may be that ts-loader just works with appropriate config. Or tweaks may be required. Please feel free to investigate; I'll assist where I can.

@johnnyreilly
Copy link
Member

I suspect this issue may be interesting to you @JasonKleban webpack/webpack#13252

@johnnyreilly
Copy link
Member

It's possible that @alexander-akait's advice here may be what you need:

webpack/webpack#13252 (comment)

Simple: we should not do this https://github.com/TypeStrong/ts-loader/blob/main/src/resolver.ts, instead we should use this.getResolve (simple example https://github.com/webpack-contrib/less-loader/blob/master/src/utils.js#L35), by default you can set extensions: [".ts", "..."], so in future developers don't need to set resolve.extension with .ts extension, it will work out of box.

Plus no problems with future major webpack versions and no need to have enhanced-resolve in deps

@andrewbranch
Copy link
Contributor

I just want to cross-reference microsoft/TypeScript#37582 which is where I redirected microsoft/TypeScript#46344 to. I would love to hear feedback there from Webpack users about

  • Why do you want/need to use .mts/.cts files?
  • Do you rely on other node12/nodenext module resolution features like export maps?
  • Do you have a mix of ESM and CJS dependencies?
  • Is anyone using Webpack’s resolve.conditionNames and why?
  • Do you want/need to write import paths with complete file extensions? Why?

@johnnyreilly
Copy link
Member

Thanks @andrewbranch - much appreciated!

@johnnyreilly
Copy link
Member

@djcsdy - feels like this might be interesting to you given your work on https://github.com/softwareventures/resolve-typescript-plugin

@alexander-akait
Copy link
Contributor

I think we don't need a plugin here, webpack already do the same for cjs/mjs, https://github.com/webpack/webpack/blob/main/lib/config/defaults.js#L490, somebody can shortly write problems places with resolving and what is expected and I will show/send a PR with implement

@andrewbranch
Copy link
Contributor

@alexander-akait the issue is that in the new TypeScript module resolution modes, users must write import "./foo.js" even though on disk, the colocated file is ./foo.ts. This is because TypeScript expects TS code to be compiled to JS with these import paths unchanged, that way the output JS will resolve correctly at runtime. But this breaks the expectations of bundlers like Webpack, which does module resolution before a 1:1 TS→JS compilation happens. So the correct specifier for Webpack would be import "./foo.ts" but TypeScript doesn’t allow that. The correct specifier for TypeScript is import "./foo.js" but Webpack can’t resolve that by default.

@alexander-akait
Copy link
Contributor

@andrewbranch Thanks

@johnnyreilly Does we support import foo from './test.ts' here before? want to understand how do not break it for other

@johnnyreilly
Copy link
Member

johnnyreilly commented Oct 14, 2021

So people don't specify extensions when they're writing TypeScript with ts-loader at present. An import would be extensionless like so:

import { faqsPath } from './misc/FAQs';

Even though the underlying file is FAQs.tsx in this case.

@alexander-akait
Copy link
Contributor

Shorty:

  1. import "./foo" -> based on webpack resolve.extensions logic (now developer uses [ts, 'tsx', ...])
  2. import "./foo.js" -> try to load foo.ts and then try to load using resolve.extensions logic
  3. import "./foo.cjs" -> try to load foo.cts and then try to load using resolve.extensions logic
  4. import "./foo.mjs" -> try to load foo.mts and then try to load using resolve.extensions logic

Also we should respect type in package.json like we do in https://github.com/webpack/webpack/blob/main/lib/config/defaults.js#L491

@alexander-akait
Copy link
Contributor

alexander-akait commented Oct 14, 2021

For better setup we can use dependencyType:

  1. typescript-auto
  2. typescript-commonjs
  3. typescript-esm

so developer can setup it better using:

resolver: {
  byDependecy: {
    'typescript-esm': {
      extensions: ['.tsx', '...']
    }
  }
}

@alexander-akait
Copy link
Contributor

alexander-akait commented Oct 14, 2021

Ideally we should go away from the common resolve.extensions option, it is really weird when developers use ['.ts', '.tsx', '.css', '.js', '.cjs', '.mjs'], because resolving logic is different for different dependencies types

@alexander-akait
Copy link
Contributor

Here only one edge case, if developer has type: 'module' before and we will implement typescript-esm logic, but he uses non extensions imports, we can break build

@alexander-akait
Copy link
Contributor

@andersekdahl @johnnyreilly And another question, is typescript exports resolution logic (like function or class), because we can emulate it here, but in future, it can be changed more, maybe we can reuse resolution logic and avoid any problems in future?

@andrewbranch
Copy link
Contributor

It’s internal API, but it looks like it’s already used here:

compiler.resolveModuleName(

@alexander-akait
Copy link
Contributor

hm, weird, it means import foo from './foo.mjs'; should work

@andrewbranch
Copy link
Contributor

I’ll give it a try this afternoon.

@alexander-akait
Copy link
Contributor

@andrewbranch Feel free to ping me, I some busy today/tomorrow, but will be free on this weekends, and look at this deeply, will be glad to any investigations so we can fix it faster

@andrewbranch
Copy link
Contributor

andrewbranch commented Oct 14, 2021

@alexander-akait I put up a super simple repro showing that .js resolution to .ts doesn’t work, but haven’t had time to start seeing how ts-loader could be involved here. Honestly my expectation was that this would not work; I’m not super familiar with how ts-loader contributes to Webpack’s resolution, but my impression was that this would be a Webpack-level thing, not a ts-loader thing. https://github.com/andrewbranch/nodenext-webpack

@alexander-akait
Copy link
Contributor

Thanks, I will look at this

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 13, 2021

@johnnyreilly We have a bug inside ts-loader, webpack resolve broken with error:

Error: context argument is not an object
    at Resolver.resolve (/home/evilebottnawi/IdeaProjects/nodenext-webpackzxzx/node_modules/enhanced-resolve/lib/Resolver.js:261:20)
    at Resolver.resolveSync (/home/evilebottnawi/IdeaProjects/nodenext-webpackzxzx/node_modules/enhanced-resolve/lib/Resolver.js:236:8)
    at /home/evilebottnawi/IdeaProjects/nodenext-webpackzxzx/node_modules/enhanced-resolve/lib/index.js:94:19
    at resolveModule (/home/evilebottnawi/IdeaProjects/nodenext-webpackzxzx/node_modules/ts-loader/dist/servicesHost.js:724:34)
    at /home/evilebottnawi/IdeaProjects/nodenext-webpackzxzx/node_modules/ts-loader/dist/servicesHost.js:123:63
    at Array.map (<anonymous>)
    at Object.resolveModuleNames (/home/evilebottnawi/IdeaProjects/nodenext-webpackzxzx/node_modules/ts-loader/dist/servicesHost.js:123:45)
    at actualResolveModuleNamesWorker (/home/evilebottnawi/IdeaProjects/nodenext-webpackzxzx/node_modules/typescript/lib/typescript.js:113491:153)
    at resolveModuleNamesWorker (/home/evilebottnawi/IdeaProjects/nodenext-webpackzxzx/node_modules/typescript/lib/typescript.js:113760:26)
    at resolveModuleNamesReusingOldState (/home/evilebottnawi/IdeaProjects/nodenext-webpackzxzx/node_modules/typescript/lib/typescript.js:113856:24)

Just put console.log https://github.com/TypeStrong/ts-loader/blob/main/src/servicesHost.ts#L1242 here, it is really very big problem, we did not encounter this often because ts resolution is very close with JS resolution logic and so webpack logic

@alexander-akait
Copy link
Contributor

Also interesting when I use "module": "nodenext", ts resolver resolves .ts file successfully, you can check it here https://github.com/TypeStrong/ts-loader/blob/main/src/servicesHost.ts#L1244...

@johnnyreilly
Copy link
Member

I don't fully understand the error to be honest.

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 13, 2021

@johnnyreilly

Solved:

import { fileURLToPath } from "url";
import { dirname, join } from "path";

class NodeNextTSPlugin {
  apply(resolver) {
    const target = resolver.ensureHook("file");
    
    resolver
        .getHook("raw-file")
        .tapAsync("NodeNextTSPlugin", (request, resolveContext, callback) => {
          const obj = {
            ...request,
            path: request.path.replace(/\.js(x?)/, '.ts$1'),
            relativePath:
                request.relativePath && request.relativePath.replace(/\.js(x?)/, '.ts$1')
          };

          resolver.doResolve(
              target,
              obj,
              this.appending,
              resolveContext,
              callback
          );
        });
  }
}

/** @type {import("webpack").Configuration} */
const config = {
  mode: "development",
  entry: "./src/main.ts",
  output: {
    path: join(dirname(fileURLToPath(import.meta.url)), "dist"),
  },
  module: {
    rules: [{ test: /\.(m|c)?ts$/, use: "ts-loader" }]
  },
  resolve: {
    plugins: [new NodeNextTSPlugin()],
    extensions: [],
  },
};

export default config;

Also we can put it inside ts-loader and allow to import it from loader (for better DX), like

import { NodeNextTSPlugin } from "ts-loader"

export default {
  // Developer options
  resolver: {
    plugins: [ new NodeNextTSPlugin() ].
  }.
};

Regarding Error: context argument is not an object, you pass invalid arguments to resolveSync (https://github.com/TypeStrong/ts-loader/blob/main/src/servicesHost.ts#L1229), first argument should be context, not undefined, just change it on const originalFileName = resolveSync(path.normalize(path.dirname(containingFile)), moduleName);

@johnnyreilly
Copy link
Member

That's fast work! I think we'd be open to a PR that added this in. However I think work is still ongoing on how nodenext etc works in TypeScript and maybe we should be holding on for that? @andrewbranch may have a view

Regarding Error: context argument is not an object, you pass invalid arguments to resolveSync

If you wanted to PR this fix now there's no reason we couldn't land it I think

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 13, 2021

@johnnyreilly Anyway I strongly recommend do not use create.sync https://github.com/TypeStrong/ts-loader/blob/main/src/resolver.ts#L8 and remove this code, Why?

  • You do extra resolve call using webpack built-in resolved, it decrease perf very well (just comment code https://github.com/TypeStrong/ts-loader/blob/main/src/servicesHost.ts#L1228-L1242) on big project and you will see how it faster, I do experiment, on one big project and removing it reduce time on 2,6 second, it is very big number
  • Mixing resolution logic between JS and TS is not good idea (yes, they are same, but if they are same why we need do extra resolution call (see before)? If developer need do own strange logic, it will be better to use loaderContext.getResolve (https://webpack.js.org/api/loaders/#thisgetresolve) with dependencyType: "typescript" and developer can set logic using:

https://webpack.js.org/configuration/resolve/#resolvebydependency

module.exports = {
  // ...
  resolve: {
    byDependency: {
      // ...
      typescript: {
        // Other options
        extensions: ['.my-strange-extension-for-ts'],
      },
    },
  },
};

also it is cachable, now we spend time on creating resolve for every ts file

  • ts does good job with resolving using tsconfig.json option, attempts to repeat the same logic on ts-loader side is not good direction.

So my solutions:

  • Сompletely remove sync webpack resolving logic, if developer really need weird resolving (I think it is bad idea and you should drink coffee and relax, and think again - do you really need this,), but if you really want to do it, we can allow define custom https://github.com/microsoft/TypeScript-wiki/blob/main/Using-the-Compiler-API.md#customizing-module-resolution, and developer can do it, we will speed up the time very very well
  • Migrate from sync webpack resolving logic on loaderContext.getResolve with dependencyType: "typescript", it will be cachable and faster than we have now, also it allow to avoid mixing logic between JS and TS

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 13, 2021

@johnnyreilly Unfortunately, I don't have enough time right now (I am working on some great rust based things, in the near future I will do tweet about it, so I didn't give the solution so quickly here), I would be glad if someone does it, I can review code, because these changes are not hard

Anyway we can put solution in README (even it doesn't finished yet), so developer can start to use extensions in their imports like in ESM

@johnnyreilly
Copy link
Member

Thanks for sharing your thoughts @alexander-akait - I don't quite follow all the things you've suggested. If you wanted to start a PR maybe I'd be able to complete it for you? Or someone else too!

@johnnyreilly
Copy link
Member

johnnyreilly commented Nov 13, 2021

Regarding Error: context argument is not an object, you pass invalid arguments to resolveSync (https://github.com/TypeStrong/ts-loader/blob/main/src/servicesHost.ts#L1229), first argument should be context, not undefined, just change it on const originalFileName = resolveSync(path.normalize(path.dirname(containingFile)), moduleName);

Are you suggesting replacing:

    const originalFileName = resolveSync(
      undefined,
      path.normalize(path.dirname(containingFile)),
      moduleName
    );

With:

const originalFileName = resolveSync(path.normalize(path.dirname(containingFile)), moduleName);

?

Сompletely remove sync webpack resolving logic, if developer really need weird resolving (I think it is bad idea

It looks like this would be a non-breaking change?

@alexander-akait
Copy link
Contributor

@johnnyreilly Yes, no breaking change, now it is just broken and do not work, put console.log and in catch and you will see error always

@djcsdy
Copy link

djcsdy commented Dec 13, 2021

fwiw @alexander-akait's code is pretty close to what @softwareventures/resolve-typescript-plugin does. I do need to add support for .mts and .cts extensions to that plugin. I'll get to it ASAP.

@johnnyreilly If you're happy to take PRs to merge this kind of functionality into ts-loader, I can help with that, and remove the need for resolve-typescript-plugin in the process :-)

@alexander-akait
Copy link
Contributor

@djcsdy Yep, anyway we can improve perf more, we don't need to rewrite it for all extensions https://github.com/softwareventures/resolve-typescript-plugin/blob/main/index.ts#L28, only for ts/cts/mts, so we can add test regexp check, you try to solve it using node_modules regexp because most of js inside there, but JS can be not only in node_modules

You register two hooks https://github.com/softwareventures/resolve-typescript-plugin/blob/main/index.ts#L24, it can be bad for perf if you have a lot of ts files, better to improve regexp and use only one hook

@djcsdy
Copy link

djcsdy commented Dec 13, 2021

Good suggestions, thank you @alexander-akait.

@johnnyreilly
Copy link
Member

@djcsdy - yup we're up for PRs! All help appreciated ❤️🌻

@vankop
Copy link

vankop commented Jun 27, 2022

this should be solved by webpack/enhanced-resolve#351


webpack will have a new option for resolve

extensionAlias: {
  ".js": [".ts", ".js"]
}

@manuth
Copy link
Contributor

manuth commented Sep 16, 2022

Temporary Workaround

In order to apply this fix, your webpack configuration must be written in JavaScript or TypeScript (e.g. not JSON or YML).

Quick Guide

I just gave it a try and temporarily changed your jsJsx and jsJsxMap to patterns which also match .cjs and .mjs files.

export const jsJsx = /\.js(x?)$/i;
export const jsJsxMap = /\.js(x?)\.map$/i;

I was able to reach this goal using sinon by adding following statements to the very top of my webpack.config.ts file:

import { createSandbox } from "sinon";
import constants = require("ts-loader/dist/constants");

let sandbox = createSandbox();
sandbox.replace(constants, "jsJsx", /\.[cm]?js(x)?$/);
sandbox.replace(constants, "jsJsxMap", /\.[cm]?js(x)?\.map/);

Then, I updated the ts-loaders test setting in my webpack.config.ts to something like this:

                test: /\.[cm]?tsx?$/,

Furthermore, I had to add the extensionAlias setting mentioned by @vankop:

        resolve: {
            extensionAlias: {
                ".js": [
                    ".js",
                    ".ts"
                ],
                ".mjs": [
                    ".mjs",
                    ".mts"
                ],
                ".cjs": [
                    ".cjs",
                    ".cts"
                ]
            }

After adding all this to my webpack-config, I was able to build my project despite the fact that it uses ESM-imports. It even compiled my .mts-file and it could be imported using a dynamic import-call from my CommonJS file.

So, as far as I can tell, replacing said constants might fix ts-loader. However, I don't know whether this change might bring unwanted behavior at other places.

Explanation

When using ESM-style imports in a typescript project, webpack will try to find .cjs, .mjs and .js filesbecause of the imports including said file extensions.

To tell webpack to look for .cts, .mts and .ts files instead, I added the extensionAlias setting which fixed this issue.

However - after doing so, I faced the issuse that ts-loader is not producing any output for my .mts file. I found out that following piece of code is the reason for this issue happening:

ts-loader/src/index.ts

Lines 474 to 481 in 69a9c23

const outputFile = outputFiles
.filter(file => file.name.match(constants.jsJsx))
.pop();
const outputText = outputFile === undefined ? undefined : outputFile.text;
const sourceMapFile = outputFiles
.filter(file => file.name.match(constants.jsJsxMap))
.pop();

As you can see, ts-loader only considers .js, .jsx, .js.map and .jsx.map files as valid output. However, compiling a .mts file produces .mjs, .mjs.map and .d.mts files.

This is why I came up with the solution to replace the regex patterns in the constants with /\.[cm]?js(x)?$/ and /\.[cm]?js(x)?\.map/. This might be quite a dirty workaround as .cjsx would be matched as well which is - if I remember correctly (?) - an incorrect file extension.

However - using this workaround, I found myself able to successfully build my TypeScript project. The .mts file and the corresponding .map file have been generated properly as well!

TL;DR Version

I think ts-loader will work perfectly with .mts and .cts files if we just change jsJsx and jsJsxMap to /\.[cm]?js(x)?$/ and /\.[cm]?js(x)?\.map/

If file extensions such as .cjsx and .mjsx have to be prevented (bc... I seriously don't know whether these are valid extensions?) the patterns would have to be adjusted a little, ofc.

@manuth
Copy link
Contributor

manuth commented Sep 16, 2022

What's still not working is importing "type": "module" dependencies for some reason I don't know.

@manuth
Copy link
Contributor

manuth commented Sep 17, 2022

@johnnyreilly I managed it to get importing ESModule dependencies to work by tweaking the serviceHost.js file a little.
Sadly, I don't really know the ts-loader that good as to be able to test my change properly.

Should I just open up a PR introducing these changes and hope a maintainer will add tests for the changes and make sure all is still working as expected?

@johnnyreilly
Copy link
Member

Do it - let's see where it goes

@manuth
Copy link
Contributor

manuth commented Sep 18, 2022

Thanks @johnnyreilly
I tried my best - hope it's working out well 😄

@fluggo
Copy link

fluggo commented Sep 29, 2022

I want to add, I've been doing a deep dive on this topic, and if all you're doing is a map of .js to .ts, you're probably going to have a bad day. Node16/NodeNext module resolution in TypeScript also specifies that you're importing CommonJS modules from ESM the way Node.js does, which is to say that if you're asking for import foo from './foo.cjs' in an ESM project, Node.js always gives you the whole module.exports object and never the default export. As far as I can tell, Webpack's output still operates by the old rules with __esModule interop in place. Please do correct me if I'm wrong.

I do not know the solution to this just yet.

@manuth
Copy link
Contributor

manuth commented Sep 29, 2022

Let's hope this won't ever be an issue, then 🥲
So far, I could compile my mixed CJS/MJS project (it's a VSCode extension, to be exact) just fine 😄

@andrewbranch
Copy link
Contributor

@fluggo that’s exactly right. Node16/NodeNext were not designed for use in a bundler (hence the name). A mode designed for bundlers is in the works.

@fluggo
Copy link

fluggo commented Sep 30, 2022

I've had some success so far using an override tsconfig.json specifying node resolution where the client files live, but that runs into trouble when I've got code that's shared between the client and the server. For now, the solution will probably be to keep asking Node to use classic-style resolution, but that feels extra janky. I want one resolution strategy that both sets of code can share.

Edit: Or, to be able to tell webpack what kind of resolution is going on.

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

Successfully merging a pull request may close this issue.

8 participants