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

extensionAlias is not applied to module exports #355

Closed
remcohaszing opened this issue Sep 13, 2022 · 21 comments · Fixed by #380
Closed

extensionAlias is not applied to module exports #355

remcohaszing opened this issue Sep 13, 2022 · 21 comments · Fixed by #380

Comments

@remcohaszing
Copy link

Let’s say we have a TypeScript mono repo with one package that depends on another. The actual TypeScript configuration is left out, because it’s not needed for this example.

webpack.config.js:

/** @type {import('webpack').Configuration} */
export default {
  entry: './packages/foo/index.ts',
  mode: 'development',
  resolve: {
    extensions: ['.js', '.ts'],
    extensionAlias: {
      '.js': ['.js', '.ts']
    }
  }
}

package.json:

{
  "type": "module",
  "workspaces": [
    "packages/*"
  ],
  "dependencies": {
    "webpack": "^5.0.0",
    "webpack-cli": "^4.0.0"
  }
}

foo/packages/package.json:

{
  "name": "foo",
  "type": "module",
  "exports": "./index.js"
}

foo/packages/index.ts:

import { bar } from 'bar'

console.log(bar)

bar/packages/package.json:

{
  "name": "bar",
  "type": "module",
  "exports": "./index.js"
}

bar/packages/index.ts:

export const bar = 'bar'

Now if we build using Webpack, it throws the following error:

~/Projects/bug$ webpack
asset main.js 2.34 KiB [emitted] (name: main)
runtime modules 274 bytes 1 module
./packages/foo/index.ts 44 bytes [built] [code generated]

ERROR in ./packages/foo/index.ts 1:0-25
Module not found: Error: Can't resolve 'bar' in '/home/remco/Projects/bug/packages/foo'
resolve 'bar' in '/home/remco/Projects/bug/packages/foo'
  Parsed request is a module
  using description file: /home/remco/Projects/bug/packages/foo/package.json (relative path: .)
    Field 'browser' doesn't contain a valid alias configuration
    resolve as module
      /home/remco/Projects/bug/packages/foo/node_modules doesn't exist or is not a directory
      /home/remco/Projects/bug/packages/node_modules doesn't exist or is not a directory
      looking for modules in /home/remco/Projects/bug/node_modules
        single file module
          using description file: /home/remco/Projects/bug/package.json (relative path: ./node_modules/bar)
            no extension
              Field 'browser' doesn't contain a valid alias configuration
              /home/remco/Projects/bug/node_modules/bar is not a file
            .js
              Field 'browser' doesn't contain a valid alias configuration
              /home/remco/Projects/bug/node_modules/bar.js doesn't exist
            .ts
              Field 'browser' doesn't contain a valid alias configuration
              /home/remco/Projects/bug/node_modules/bar.ts doesn't exist
        existing directory /home/remco/Projects/bug/node_modules/bar
          using description file: /home/remco/Projects/bug/node_modules/bar/package.json (relative path: .)
            using exports field: ./index.js
              using description file: /home/remco/Projects/bug/node_modules/bar/package.json (relative path: ./index.js)
                no extension
                  Field 'browser' doesn't contain a valid alias configuration
                  /home/remco/Projects/bug/node_modules/bar/index.js doesn't exist
                .js
                  Field 'browser' doesn't contain a valid alias configuration
                  /home/remco/Projects/bug/node_modules/bar/index.js.js doesn't exist
                .ts
                  Field 'browser' doesn't contain a valid alias configuration
                  /home/remco/Projects/bug/node_modules/bar/index.js.ts doesn't exist
                as directory
                  /home/remco/Projects/bug/node_modules/bar/index.js doesn't exist
      /home/remco/Projects/node_modules doesn't exist or is not a directory
      /home/remco/node_modules doesn't exist or is not a directory
      /home/node_modules doesn't exist or is not a directory
      /node_modules doesn't exist or is not a directory

This shows that extensionAlias isn’t applied when resolving import 'bar' from the foo project. This can be worked around by removing the exports field from package.json, but that will cause Node.js to show deprecation warnings if the package would be published and consumed.

@alexander-akait
Copy link
Member

Hello, sorry for delay, currently it is expected, because exports should work as written (i.e. if you requested file.js we should return file.js and if you requested file.ts we should return index.ts).

But this case is intresting (especially since module request can only be done for Node.js), to be honest I don't have strong opinion here, on the one hand, violation of the specification for this would be a completely wrong decision, but on the other side I see your problem and what you are trying to solve (monorepo and typescript).

Based on the logic of modularity and forget about mono-repositories, that is, each package is in its own repository and requires publication (i.e. npm publish) in the registry before using it, then you would anyway need to publish js files (i.e. compile/transpile them) and have the exports with js files (otherwise developer without typescript can't consume your library). Yes, we have the extensionAlias option, but they are required a little for another problem and was not design to change exports extensions.

Also If we allow to do it, some developers can start to rely on this and create a situation when package can't be used without webpack, that is really bad.

So I think we're doing the right thing here.

I see these solution (and consider them valid as for any bundler as for Node.js itself)

  1. Using import foo from "package/index.ts"
  2. The subpath solution:
{
  "type": "module",
  "exports": {
    ".": "./index.js",
    "./ts": "./index.ts"
  }
}

So you can use import foo from "foo/ts";, but other developers can stoll consume your library using import foo from "foo";

  1. The condition exports https://nodejs.org/api/packages.html#conditional-exports - add typescript section when set them for webpack resolver
  2. Alternative - create an own plugin for resolver (it will be not hard)

@remcohaszing
Copy link
Author

I see what you mean, but I don’t consider this to be a violation of the spec, but parity with the rest of the ecosytem.

As can be seen in the output I posted, Webpack already tries to apply the extensions defined in resolve.extensions to the resolved import. This would be the same type of violation.

using description file: /home/remco/Projects/bug/node_modules/bar/package.json (relative path: ./index.js)
  no extension
    Field 'browser' doesn't contain a valid alias configuration
    /home/remco/Projects/bug/node_modules/bar/index.js doesn't exist
  .js
    Field 'browser' doesn't contain a valid alias configuration
    /home/remco/Projects/bug/node_modules/bar/index.js.js doesn't exist
  .ts
    Field 'browser' doesn't contain a valid alias configuration
    /home/remco/Projects/bug/node_modules/bar/index.js.ts doesn't exist
  as directory
    /home/remco/Projects/bug/node_modules/bar/index.js doesn't exist

TypeScript’s node16 module resolution, understands that the referenced .js file referenced in package exports field should be resolved to the .ts file instead. Also ts-node understands this.

A very similar issue has been acknowledged in privatenumber/tsx#59.

I really appreciate you’re thinking along, but this workaround won’t always work:

{
  "type": "module",
  "exports": {
    ".": "./index.js",
    "./ts": "./index.ts"
  }
}

Let’s say we have package a, which is published to npm, package b, which depends on a and is also published to npm, and package c, which is private and built using Webpack. Now package b would need to import a in case it’s published to npm, and a/ts when it’s consumed by c.

@alexander-akait
Copy link
Member

I think the problem is still unresolved (in esbuild and for us too) because it creates a situation where users start writing:

{
  "type": "module",
  "exports": {
    ".": "./index.ts"
  }
}

Because they will expect mapping ts(tsx) to js, but it is fully wrong...

@alexander-akait
Copy link
Member

alexander-akait commented Sep 16, 2022

hm, Intresting, if you have:

  "exports": {
    ".": "./index.js",
     "./main.js": "./main.js"
  }

and use import foo from "foo/main.js";, we apply extensionAlias logic, but output an error here, because it was not exports - i.e. Package path ./main.ts is not exported from package and we need to output this and it is valid - you cannot import anything that is not in the exports field.

So you need:

  "exports": {
    ".": "./index.js",
    "./main.ts": "./main.js",
    "./main.js": "./main.ts"
  }

And I think you see the problem here.

Also Looking at code and the Node.js logic (and not only), I still think you should use condition exports here, otherwise you will get something like above - we can't resolve file if don't have it in the exports file.

@vankop @sokra What do you think?

@vankop
Copy link
Member

vankop commented Oct 3, 2022

I fully agree with @alexander-akait idea => exports should work as written.

btw this seems wrong to me:

using exports field: ./index.js
              using description file: /home/remco/Projects/bug/node_modules/bar/package.json (relative path: ./index.js)
                no extension
                  Field 'browser' doesn't contain a valid alias configuration
                  /home/remco/Projects/bug/node_modules/bar/index.js doesn't exist
                .js
                  Field 'browser' doesn't contain a valid alias configuration
                  /home/remco/Projects/bug/node_modules/bar/index.js.js doesn't exist
                .ts
                  Field 'browser' doesn't contain a valid alias configuration
                  /home/remco/Projects/bug/node_modules/bar/index.js.ts doesn't exist
                as directory
                  /home/remco/Projects/bug/node_modules/bar/index.js doesn't exist

we should not apply extensions..

@alexander-akait
Copy link
Member

@vankop Yeah we should decide here - should we apply extensionAlias/extensions here or not

@sokra Need you advice

@wesselvdv
Copy link

For anyone who's also running into this issue, I've fixed this using alternatives as described in the webpacks docs(https://webpack.js.org/guides/package-exports/#alternatives):

  "exports": {
    ".": ["./src/index.js", "./src/index.ts"],
    "./*": ["./src/*/index.js", "./src/*.js", "./src/*/index.ts", "./src/*.ts"]
  },

I couldn't find if this is according to the esm spec, but it also works with tsc.

@Alxandr
Copy link

Alxandr commented Apr 27, 2023

Just encountered this issue with a slightly different setup. I have the following in my exports:

	"exports": {
		".": {
			"import": "./dist/index.js"
		},
		"./runtime.js": {
			"import": "./dist/runtime.mjs"
		}
	},

And I'm doing import * as __i18n from "@nmid/i18n/runtime.js"; which results in:

Package path ./runtime.ts is not exported from package.

I'd argue this is definitely wrong, and I have no idea why it's trying to import a typescript file here. runtime.ts does not exist in the package folder even. It's under src.

@alexander-akait
Copy link
Member

alexander-akait commented Apr 27, 2023

Please provide example of the problem

@raviqqe
Copy link

raviqqe commented Apr 27, 2023

I made a minimal reproducible one of the issue here. https://github.com/raviqqe/til/tree/main/webpack-exports-extension-alias

@alexander-akait
Copy link
Member

alexander-akait commented Apr 27, 2023

You have:

 "exports": {
    "./*.js": {
      "types": "./dist/*.d.ts",
      "default": "./dist/*.js"
    }
  },

And what you expected? Typescript code? @raviqqe/hidash doesn't have typescript code, look at package structure, I see only string.test.d.ts and it is export {};, it is not code.

Also types is special thing for typescript, you have two thing wrong:

  1. "./*.js" - you says - you allow to load only js files, change it to "./*"
  2. If you want to import types you need to use @import something from "@raviqqe/hidash/types/string" or for regular export @import something from "@raviqqe/hidash/string"

If you want to use .js at the end you need to create two entrypint in export - for regular js and types, it is not my rules, it is how exports works

@raviqqe
Copy link

raviqqe commented Apr 27, 2023

"./.js" - you says - you allow to load only js files, change it to "./"

It's just to load JS files. So it doesn't matter.

If you want to import types you need to use @import something from "@raviqqe/hidash/types/string" or for regular export @import something from "@raviqqe/hidash/string"

The types field is for TypeScript's type system to load types of the JS files automatically.

Both usage is described in the Node.js's documentation (https://nodejs.org/api/packages.html#subpath-exports). I'm not sure if I'm using it correctly but at least it works with the latest TypeSript's module resolution strategy of node16 and nodenext.

@alexander-akait Are you really sure that the usage is wrong? It's literally written in the official documentation.

This is also a good read to understand how TypeScript uses exports fields. microsoft/TypeScript#33079

@alexander-akait
Copy link
Member

alexander-akait commented Apr 27, 2023

@alexander-akait Are you really sure that the usage is wrong? It's literally written in the official documentation.

Where? I mean you can use */*.js, but if you want to export typescript code (not just types) and mapping it, you can't, you should avoid using extensions so extensionAlias can check all possible extensions

The types conditional field doesn't used by bundlers, it is for types checking tools, webpack is not type checking tool, so I really can't undestand what you try to achive and how it related to extensionAlias

You can add this field to https://webpack.js.org/configuration/resolve/#resolveconditionnames and webpack will resolve files from the types field, but again, what is the point to do it

@raviqqe
Copy link

raviqqe commented Apr 27, 2023

Where?

Here.

I think there is a fundamental misunderstanding here about how TypeScript's new module resolution (node16 and nodenext) work with ES modules. In the cases of @Alxandr and mine, we are just trying to resolve JS files from the external packages with exports fields with Webpack as you suggested. The reason we need to use the extensionAlias option is that the TypeScript compiler doesn't transpile import paths from .ts to .js for fully qualified imports in ESM, which is how the TS team decided to do (microsoft/TypeScript#35589, microsoft/TypeScript#40878.)

@alexander-akait
Copy link
Member

I think there is a fundamental misunderstanding here about how TypeScript's new module resolution (node16 and nodenext) work with ES modules.

Is it? I implemented this logic a long time ago TypeStrong/ts-loader#1383 (comment), when typescript realses it, it seems to me that you do not want to listen to me

Again - you have a typescript file and want to use extensionAlias, but file IN conditinalName field and has nothing to do with extensionAlias option, so if you want to support types, you need to set https://webpack.js.org/configuration/resolve/#resolveconditionnames, by default webpack for javascript and know nothing about typescript/coffeescript/flow/etc, so loader like ts-loader should create an own resolver and provide additional fields in conditinalNames, example sass-loader - https://github.com/webpack-contrib/sass-loader/blob/master/src/utils.js#L533 add sass and style fields, webpack knows nothing about how CSS works and how to resolve it.

@raviqqe
Copy link

raviqqe commented Apr 28, 2023

Yes, you do. 😭

so if you want to support types, you need to set https://webpack.js.org/configuration/resolve/#resolveconditionnames,

No, I'm not talking about supporting the types conditional fields... I just brought up the use of extensionAlias for TypeScript because it's one of the primary use cases. The bug actually has nothing to do with TypeScript. As I mentioned before, we are not trying to resolve TypeScript files with the types condition names in Webpack. The statement I'm making is nothing to do with the condition names.

I made my reproducible example TS-free (but with Lisp :) to help your understanding of the bug.

Now, the error message looks like this with extensionAlias of ".js": [".lisp", ".js"]:

> npm run build

> webpack-exports-extension-alias@1.0.0 build
> webpack

assets by status 122 bytes [cached] 1 asset
./src/main.js 43 bytes [built] [code generated]

ERROR in ./src/main.js 1:0-42
Module not found: Error: Package path ./string.lisp is not exported from package /Users/raviqqe/src/github.com/raviqqe/til/webpack-exports-extension-alias/node_modules/@raviqqe/hidash (see exports field in /Users/raviqqe/src/github.com/raviqqe/til/webpack-exports-extension-alias/node_modules/@raviqqe/hidash/package.json)

webpack 5.80.0 compiled with 1 error in 86 ms

@alexander-akait
Copy link
Member

alexander-akait commented Apr 28, 2023

Now I see, yeah, it is a bug, I thought that the main idea is to import it, but here it’s the other way around, the idea is not to try to import ts file

@alexander-akait alexander-akait moved this to Priority - High in webpack 5/6 Apr 28, 2023
@alexander-akait
Copy link
Member

The fix is easy, but we need to do deep test with it (I will do soon, hope this week), because ignoring errors can be a problem in some cases when you have extensionAlias

@Alxandr
Copy link

Alxandr commented Apr 28, 2023

In case someone else arrives here with this issue before it's solved, and need a work-around, I ended up just using a non-aliased file-extension in my exports:

	"exports": {
		".": {
			"import": "./dist/index.js"
		},
		"./runtime.mjs": { // <- mjs is not aliased in my project.
			"import": "./dist/runtime.mjs" // <- the extension here does not need to be the same as the line above (even though it is in this case)
		}
	},

[Edit]:
I would also generally recomend against using * in exports, but that's an aside not really related to this issue. Just saw a lot of it posted here.

@alexander-akait
Copy link
Member

@Alxandr * exports were fixed recently, now we use the same logic as in Node.js source code, exactly the same code, so using * is safe to use

@Alxandr
Copy link

Alxandr commented May 8, 2023

@alexander-akait it was not a "don't use it cause it's unsafe". It was just a general in my honest opinion, it's a bad idea to use * exports, cause you make the entire surface of your package public API.

@github-project-automation github-project-automation bot moved this from Priority - High to Ready for Merge in webpack 5/6 May 8, 2023
@TheLarkInn TheLarkInn moved this from Ready for Merge to Shipped in webpack 5/6 May 8, 2023
@TheLarkInn TheLarkInn moved this from Shipped to Need's Release in webpack 5/6 May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Shipped
Development

Successfully merging a pull request may close this issue.

6 participants