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

Inconsistency with TypeScript in Module Resolution with Fully-Specified ESM Imports #13252

Closed
Josehower opened this issue Apr 28, 2021 · 53 comments

Comments

@Josehower
Copy link

Josehower commented Apr 28, 2021

Bug report

What is the current behavior?

When using extention .js as part of an import (fully-specified ESM import), webpack fails to create a bundle if the file imported have extention .tsx

This shouldnt be like this based on the comment of RyanCavanaugh
in this issue microsoft/TypeScript#41887 (comment)

"TypeScript doesn't modify JavaScript code you write, the import path you write should be the one you want to appear in the output .js file"

If the current behavior is a bug, please provide the steps to reproduce.

I created a repo showing this unexpected behavior
this repo was tested with both ts-loader and babel-loader

https://github.com/Josehower/webpack-babel-test

  1. clone the repo and install dependencies
  2. use command $yarn start

image

import { test } from './test.js';

console.log('Hello Webpack Project.');
test();
jose fernando hower@DESKTOP-FHTIMFT MINGW64 /D/upleveled-softwork/webpack-test (main)
$ yarn start
yarn run v1.22.5
$ webpack serve --config ./webpack.config.js --mode development
i 「wds」: Project is running at http://localhost:8080/
i 「wds」: webpack output is served from /
i 「wds」: Content not from webpack is served from D:\upleveled-softwork\webpack-test\dist
× 「wdm」: asset bundle.js 367 KiB [emitted] (name: main)
runtime modules 706 bytes 4 modules
cacheable modules 336 KiB
  modules by path ./node_modules/webpack-dev-server/ 21.2 KiB
    modules by path ./node_modules/webpack-dev-server/client/ 20.9 KiB 10 modules
    modules by path ./node_modules/webpack-dev-server/node_modules/ 296 bytes 2 modules
  modules by path ./node_modules/html-entities/lib/*.js 61 KiB 5 modules
  modules by path ./node_modules/querystring/*.js 4.51 KiB 3 modules
  modules by path ./node_modules/webpack/hot/ 1.42 KiB
    ./node_modules/webpack/hot/emitter.js 75 bytes [built] [code generated]
    ./node_modules/webpack/hot/log.js 1.34 KiB [built] [code generated]
  modules by path ./node_modules/url/*.js 23.1 KiB
    ./node_modules/url/url.js 22.8 KiB [built] [code generated]
    ./node_modules/url/util.js 314 bytes [built] [code generated]
./node_modules/webpack/hot/ sync nonrecursive ^\.\/log$ 170 bytes [built] [code generated]

ERROR in ./src/index.tsx 1:0-33
Module not found: Error: Can't resolve './test.js' in 'D:\upleveled-softwork\webpack-test\src'
resolve './test.js' in 'D:\upleveled-softwork\webpack-test\src'
  using description file: D:\upleveled-softwork\webpack-test\package.json (relative path: ./src)
    Field 'browser' doesn't contain a valid alias configuration
    using description file: D:\upleveled-softwork\webpack-test\package.json (relative path: ./src/test.js)
      no extension
        Field 'browser' doesn't contain a valid alias configuration
        D:\upleveled-softwork\webpack-test\src\test.js doesn't exist
      *
        Field 'browser' doesn't contain a valid alias configuration
        D:\upleveled-softwork\webpack-test\src\test.js* doesn't exist
      .js
        Field 'browser' doesn't contain a valid alias configuration
        D:\upleveled-softwork\webpack-test\src\test.js.js doesn't exist
      .ts
        Field 'browser' doesn't contain a valid alias configuration
        D:\upleveled-softwork\webpack-test\src\test.js.ts doesn't exist
      .tsx
        Field 'browser' doesn't contain a valid alias configuration
        D:\upleveled-softwork\webpack-test\src\test.js.tsx doesn't exist
      as directory
        D:\upleveled-softwork\webpack-test\src\test.js doesn't exist

webpack 5.36.0 compiled with 1 error in 2410 ms
i 「wdm」: Failed to compile.
  1. use command $yarn tsc --noEmit
jose fernando hower@DESKTOP-FHTIMFT MINGW64 /D/upleveled-softwork/webpack-test (main)
$ yarn tsc --noEmit
yarn run v1.22.5
$ D:\upleveled-softwork\webpack-test\node_modules\.bin\tsc --noEmit
✨  Done in 1.49s.

image

What is the expected behavior?

  • In the example repo yarn start should succesfully create a bundle.

  • Webpack should be able to recognize the files imported and create a bundle even if the import statements with the extention .js point files that have the extention .tsx

Other relevant information:

OS: Windows 10 10.0.19041
CPU: (4) x64 Intel(R) Core(TM) i5-7400 CPU @ 3.00GHz
Binaries:
Node: 16.0.0 - ~\AppData\Local\Temp\yarn--1619542324765-0.4257021548777624\node.CMD
Yarn: 1.22.5 - ~\AppData\Local\Temp\yarn--1619542324765-0.4257021548777624\yarn.CMD
npm: 7.10.0 - C:\Program Files\nodejs\npm.CMD
Browsers:
Chrome: 90.0.4430.93
Edge: Spartan (44.19041.906.0), Chromium (90.0.818.46)
@karlhorky
Copy link
Contributor

karlhorky commented Apr 28, 2021

Alternately, in case @sokra or others decide that parity with TypeScript's module resolution algorithm with fully-specified ES Modules imports is not a goal of webpack, this bug / feature request may actually be better placed in the two loaders that were used to consume TypeScript (and which failed to replicate the TypeScript module resolution algorithm):

@karlhorky
Copy link
Contributor

Hmm... as a workaround in the meantime, would it be possible to create a map of ./file.js to ./file module file paths using something like the NormalModuleReplacementPlugin? 🤔

@alexander-akait
Copy link
Member

What is problem with webpack here? If you use import you need to write extension, or disable fully-specified ESM import, don't know what we should fix here...

@karlhorky
Copy link
Contributor

karlhorky commented Apr 28, 2021

The problem is that using the fully-specified .js extension fails when using TypeScript .ts files (they are not resolved in parity with the behavior of TypeScript and tsc)

With TypeScript and tsc, the following works:

// index.ts
import {abc} from './abc.js';
// abc.ts
export const abc = 1;

With webpack, it fails and cannot resolve the ./abc.js specifier (note the difference between the extensions - .js and .ts).

See @RyanCavanaugh's comment here: microsoft/TypeScript#41887 (comment)

TypeScript doesn't modify JavaScript code you write, the import path you write should be the one you want to appear in the output .js file

@alexander-akait
Copy link
Member

I think related TypeStrong/ts-loader#1198

@karlhorky
Copy link
Contributor

Maybe somewhat related, although it seems this person is describing the opposite: they want to write node-style specifiers (without the extension) and have them resolved to the .js files (ignoring the .ts file).

What we're looking for is to be able to write standard imports using ES Modules fully-specified import paths (with .js extension) and that they be resolved to the .ts files (as TypeScript + tsc do today)

@alexander-akait
Copy link
Member

Give me time to look at this. I think problem somewhere here https://github.com/TypeStrong/ts-loader/blob/main/src/resolver.ts

@alexander-akait
Copy link
Member

Anyway why do you want to use import { test } from './test.js'; instead import { test } from './test.ts';? I think in ts better to have ts extension at the end, it is fully-specified import

@karlhorky
Copy link
Contributor

I think in ts better to have ts extension at the end, it is fully-specified import

The TypeScript team doesn't agree, check out the issue I linked above: microsoft/TypeScript#41887 (comment)

@alexander-akait
Copy link
Member

alexander-akait commented Apr 28, 2021

Yep, I see...

[tsl] ERROR in /home/path/to/webpack-babel-test/src/index.tsx(1,22)
      TS2691: An import path cannot end with a '.tsx' extension. Consider importing './test.js' instead.

We need improve it on ts-loader side, webpack works correctly here.

@karlhorky
Copy link
Contributor

Thanks for the confirmation, I've opened an issue in babel-loader over here: babel/babel-loader#904

@alexander-akait
Copy link
Member

alexander-akait commented Apr 28, 2021

Simple workaround for your simple case:

resolve: {
    alias: {
      './test.js': './test'
    },
    extensions: ['.tsx', '.ts', '...'],
},

Ideally ts-loader should respect resolve.extensions (better resolve.byDependency.typescript) and when you using import something from './foo.js' try to resolve ./foo.js as ./foo.tsx/./foo.ts and then other variants (i.e. ./foo.js and etc).

To avoid collisions between ts-loader resolving and Node.js resolving we should support https://webpack.js.org/configuration/resolve/#resolvebydependency, so you will use:

resolve: {
  byDependency: {
    typescript: {
      extensions: ['.tsx', '.ts', '...'],
    },
}    

It will be implement using https://webpack.js.org/api/loaders/#thisgetresolve, we use the same logic for sass/less/etc so @import with CSS and SASS works good.

Honestly it's not hard to do.

@alexander-akait
Copy link
Member

/cc @johnnyreilly make sense to look at this

@alexander-akait
Copy link
Member

alexander-akait commented Apr 28, 2021

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

@Josehower
Copy link
Author

Josehower commented Apr 28, 2021

another workaround I found was using NormalModuleReplacementPlugin to replace the problematic extension

 module.exports = function (env) {
  return {
    plugins: [
      new webpack.NormalModuleReplacementPlugin(new RegExp(/^\..+\.js$/), function (resource) {
        resource.request = resource.request.replace(new RegExp(/\.js$/), '');
      }),
        }
      ),
    ],
  };
};

in our case, this did the trick

Updated with the RegExp suggested by @laverdet #13252 (comment)

@alexander-akait
Copy link
Member

Yep, should works too, anyway will be better to solve it in ts-loader, because we have more profit here like better DX, resolvgin by dependency, no need enhanced-resolve in deps

@johnnyreilly
Copy link
Contributor

Yeah I think ts-loader would be open to a PR that tackles this.

@sokra
Copy link
Member

sokra commented Apr 28, 2021

Something like that could be solved in a custom resolver plugin. I think think this typescript-specific problem fits into the webpack core.

The resolver plugin could take an extensions argument which is a list of typescript extensions that should be tried when the request ends with .js.

@johnnyreilly
Copy link
Contributor

Cool - perhaps that's a better place for it to go!

@webpack-bot
Copy link
Contributor

This issue had no activity for at least three months.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@karlhorky
Copy link
Contributor

not stale

@karlhorky
Copy link
Contributor

Would also be good to document in this thread here what version of webpack will have this change, if possible.

@alexander-akait
Copy link
Member

Accidentally close, yes, you can specify extensionAlias in the resolve option (https://webpack.js.org/configuration/resolve/), like you do it for alias/etc

@vankop
Copy link
Member

vankop commented Jun 29, 2022

technically we can add this to default rules but webpack doesn't support ts out of the box.. so probably you will need specify extension alias

@karlhorky
Copy link
Contributor

karlhorky commented Jun 30, 2022

Alright, so the configuration is going to be something like this I assume:

module.exports = {
  resolve: {
    extensionAlias: {
      '.js': ['.ts', '.js'],
      '.mjs': '.mts',
      '.cjs': '.cts',
    },
  },
}

One thing that's not quite clear about this is the inconsistent inclusion of the non-transformed extensions. In the first line, the .js configuration also includes .js as a fallback. However, the .mjs and .cjs string values do not contain the non-transformed extensions.

So a more consistent configuration would be like this below (every value also including the non-transformed extensions):

module.exports = {
  resolve: {
    extensionAlias: {
      '.js': ['.ts', '.js'],
      '.mjs': ['.mts', '.mjs'],
      '.cjs': ['.cts', '.cjs'],
    },
  },
}

However, ideally the non-transformed extension would be automatically included as a fallback in all configurations, so that this would work (no arrays with non-transformed extensions needed anywhere):

module.exports = {
  resolve: {
    extensionAlias: {
      '.js': '.ts',
      '.mjs': '.mts',
      '.cjs': '.cts',
    },
  },
}

@laverdet
Copy link

laverdet commented Jul 15, 2022

another workaround i found was using NormalModuleReplacementPlugin to replace the problematic extention

 module.exports = function (env) {
  return {
    plugins: [
      new webpack.NormalModuleReplacementPlugin(new RegExp(/\.js$/), function (resource) {
        resource.request = resource.request.replace('.js', '');
      }),
        }
      ),
    ],
  };
};

in our case this did the trick

I tweaked the RegExp here because otherwise imports to modules like sha.js will fail. Thanks!

            new NormalModuleReplacementPlugin(/^\..+\.js$/, resource => {
                resource.request = resource.request.replace(/\.js$/, "");
            }),

@alexander-akait
Copy link
Member

Yeah, sometimes I think npm should not allow to use . in a package name, because it is pain to write regexp and code looks misleading:

var shajs = require('sha.js')

Do we load ./sha.js or ./sha.js/index.js? weird

@karlhorky
Copy link
Contributor

karlhorky commented Jul 31, 2022

Would also be good to document in this thread here what version of webpack will have this change, if possible.

Looks like this is in version v5.74.0

@iamWing
Copy link

iamWing commented Aug 10, 2022

another workaround i found was using NormalModuleReplacementPlugin to replace the problematic extention

 module.exports = function (env) {
  return {
    plugins: [
      new webpack.NormalModuleReplacementPlugin(new RegExp(/\.js$/), function (resource) {
        resource.request = resource.request.replace('.js', '');
      }),
        }
      ),
    ],
  };
};

in our case this did the trick

I tweaked the RegExp here because otherwise imports to modules like sha.js will fail. Thanks!

            new NormalModuleReplacementPlugin(/^\..+\.js$/, resource => {
                resource.request = resource.request.replace(/\.js$/, "");
            }),

The regex on the tweaked version doesn't seem right to me. Neither _/app.js or sha.js matches the expression.

My suggestion would be use /.*\/+.+\.js$/ instead. Matching a / character in the middle of the path can help identifying if we're importing a local module or not, because if we're importing a dependency installed we usually just write import something from 'something';, but if we're importing a local module, we write import something from './path/something.js'.

A better way to cover all cases would be tweaking the function in NormalModuleReplacementPlugin to exclude some patterns for the edge cases.

e.g.

new NormalModuleReplacementPlugin(/.*\/+.+\.js$/, (resource) => {
  if (/^sha\.js$/.test(resource.request) return; // - Exclude the edge cases using if statement
  resource.request = resource.request.replace(/\.js$/, '');
}),

Maybe we can also develop a plugin to do what we're doing with NormalModuleReplacementPlugin here but also check the dependency list from package.json so that all packages listed there can be excluded automatically? I'll do some research on this later.

Edited for typo

@karlhorky
Copy link
Contributor

karlhorky commented Aug 10, 2022

@iamWing the /^\..+\.js$/ regex seems to work for ./app.js, and it is intended to filter out sha.js:

> /^\..+\.js$/.test('./app.js')
> true

@iamWing
Copy link

iamWing commented Aug 10, 2022

@iamWing the /^\..+\.js$/ regex seems to work for ./app.js, and it is intended to filter out sha.js:

> /^\..+\.js$/.test('./app.js')
> true

@karlhorky @laverdet

My bad for the typo, I actually mean _/app.js instead of ./app.js there.

The reason I'm using _/app.js rather then ./app.js there is because I have path alias configured in tsconfig and I think a lot of people do rather than just typing long paths for relative imports, hence I proposed to check for the / in the import path to check if it's a realative import instead of ./.

@karlhorky
Copy link
Contributor

I've also opened a corresponding issue in Next.js, since it seems that they have their own flavor of the problem:

@alexander-akait
Copy link
Member

@karlhorky Can you look at the latest version?

@karlhorky
Copy link
Contributor

karlhorky commented Nov 3, 2022

Oh sorry, deleted my comment now! Indeed the type is there in 5.74.0, somehow one of my webpack versions was locked to 5.73.0 😬

@alexander-akait
Copy link
Member

I think we can close it, now we have extensionAlias - https://webpack.js.org/configuration/resolve/#resolveextensionalias

Venryx added a commit to Venryx/tree-grapher that referenced this issue Sep 16, 2023
…as problem since that's what typescript wants for transpilation; for more info, see: webpack/webpack#13252)

* Added UI (and example data) for equivalent of "timeline system", where as you progress in time, different nodes get "focused". (prepwork for a scroll-and-zoom system within tree-grapher)
@karlhorky
Copy link
Contributor

karlhorky commented Nov 23, 2023

Use extensionAlias in Next.js via experimental.extensionAlias config introduced in vercel/next.js#45423:

next.config.js

/** @type {import("next").NextConfig} */
const nextConfig = {
  experimental: {
    // Remove .js from import specifiers, because
    // Next.js and webpack do not yet support
    // TypeScript-style module resolution out of the box
    // https://github.com/webpack/webpack/issues/13252#issuecomment-1171080020
    // https://github.com/vercel/next.js/pull/45423
    // https://github.com/vercel/next.js/issues/58805
    // https://github.com/vercel/next.js/issues/54550
    extensionAlias: {
      '.js': ['.ts', '.tsx', '.js', '.jsx'],
      '.jsx': ['.tsx', '.jsx'],
      '.mjs': ['.mts', '.mjs'],
      '.cjs': ['.cts', '.cjs'],
    },
  },
}

module.exports = nextConfig

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

No branches or pull requests

9 participants