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

File Extension Substitution for TypeScript Support #413

Open
magnusriga opened this issue Mar 14, 2024 · 19 comments
Open

File Extension Substitution for TypeScript Support #413

magnusriga opened this issue Mar 14, 2024 · 19 comments

Comments

@magnusriga
Copy link

magnusriga commented Mar 14, 2024

I am trying to get the enhanced resolver to work with a TypeScript self-reference, both via package.json exports and imports.

When TypeScript looks up modules it deviates from the Node resolver once a path has been found. Specifically, it conducts what is called File Extension Substitution where it takes the path it found from package.json and swaps out the extension with .ts, .tsx, .d.ts, .js. and .jsx, in that order.

It seems enhanced resolver does not do that kind of substitution, is that right? Any way to make it mimic TypeScript's resolution algorithm?

Note: The import resolution works when we change the extensions to .tsx in the exports and imports, but the idea is that the resolver should do extension substitution when it looks for the TS files.

Minimal repo: https://github.com/magnusriga/my-app

Code excerpts follow below.

Importing file:

/* eslint-disable */
const resolve = require("enhanced-resolve");

const myResolve = resolve.create({
  conditionNames: [
    'types',   'import',
    'esm2020', 'es2020',
    'es2015',  'require',
    'node',    'node-addons',
    'browser', 'default'
  ],
  extensions: [
    '.ts',   '.tsx',
    '.d.ts', '.js',
    '.jsx',  '.json',
    '.node'
  ],
  extensionAlias: {
    '.js': [ '.ts', '.tsx', '.d.ts', '.js' ],
    '.jsx': [ '.tsx', '.d.ts', '.jsx' ],
    '.cjs': [ '.cts', '.d.cts', '.cjs' ],
    '.mjs': [ '.mts', '.d.mts', '.mjs' ]
  },
});

// Looking up local file (self reference) via exports
myResolve(__dirname, "my-app/baz", (err, result) => {
  const res1 = result;
  console.log('Resolving my-app/baz :>> ', res1);
});

// Looking up local file (self reference) via imports
myResolve(__dirname, "#app/baz", (err, result) => {
  const res2 = result;
  console.log('Resolving #app/baz :>> ', res2);
});

Local package.json

{
  "imports": {
    "#app/*": "./src/app/*.js" // Works if extension is .tsx
  },
  "exports": {
    "./*": "./src/app/*.js" // Works if extension is .tsx
  },
}
export const Baz = ({baz}: {baz: string}) => {
  return <div>{baz}</div>
}
@alexander-akait
Copy link
Member

alexander-akait commented Mar 25, 2024

Hello, sorry for delay, honestly, I'm not sure that we should work like this, I see what you want to solve, the specification is quite clear about what you are asking cannot be done. If you want to expose tsx you should use condition names, so if you have .js file we should load .js file

@magnusriga
Copy link
Author

magnusriga commented Mar 25, 2024

Hi @alexander-akait . Thanks for the response. TypeScript's official module resolution works that way though (see Extension Substitution in their official docs). If it isn't supported by enhanced-resolve, it would be necessary to make an identical package that mirrors how TS does resolution. TS's module resolution directly mirrors that of Node.js, it only deviates when a path has been found in package.json.

Is it still out of scope? When you say that the specification does not support my request, is it because the enhanced-resolve package only should implement Node.js resolution, without additional options?

@alexander-akait
Copy link
Member

Please read #355, because we can't get files which were not exported, it is fully invalid (in my opinion) and literally breaks the meaning of the exports field, because using some exotic/custom options I can get any files/modules from a package, but in fact I don’t know if the author really wanted to export them.

I am ready for dialogue on this issue once again, I would like to hear the developers' opinions here of typescript and Node.js

/cc @andrewbranch Perhaps you can ping here the person responsible for this or clarify the logic

@magnusriga
Copy link
Author

magnusriga commented Mar 30, 2024

Please read #355, because we can't get files which were not exported, it is fully invalid (in my opinion) and literally breaks the meaning of the exports field, because using some exotic/custom options I can get any files/modules from a package, but in fact I don’t know if the author really wanted to export them.

I am ready for dialogue on this issue once again, I would like to hear the developers' opinions here of typescript and Node.js

/cc @andrewbranch Perhaps you can ping here the person responsible for this or clarify the logic

I understand, but in my view Extension Substitution does not equal exporting files that the developer did not mean to export. foo.ts is just the TS version of foo.js. The file name is identical. Indeed, @andrewbranch would probably be the right person to provide color here...

@alexander-akait
Copy link
Member

alexander-akait commented Apr 2, 2024

I understand, but in my view Extension Substitution does not equal exporting files that the developer did not mean to export. foo.ts is just the TS version of foo.js. The file name is identical.

Yeah, I fully undestand you, but as before I said this is not entirely true from a specification point of view, so yes, I would like to hear @andrewbranch here and how they implemented this.

As you can see you need to use the types condition name for d.ts, so maybe we should use the typescript condition export here, so developer can export:

  • only types using the types name
  • typescript code using the typescript name

Technically a library can export any files and if we think correctly, we should do this under different conditional names...

But let's dicussion about it with ts team, I am fine to change logic to be align with them, just want that we look at solving the problem in the same way

@andrewbranch
Copy link

I’d like to understand what real-world scenario @magnusriga is trying to achieve, before I answer the theoretical design questions.

@magnusriga
Copy link
Author

magnusriga commented Apr 2, 2024

I’d like to understand what real-world scenario @magnusriga is trying to achieve, before I answer the theoretical design questions.

Hi @andrewbranch, thanks for chiming in. My question stems from the eslint-import-resolver-typescript, which relies on enhanced-resolve. It is not possible for that ESLint plugin to mirror Typescript's resolution, without changes to enhanced-resolve itself (enabling extension substitution). So, if the package.json exports field maps to .js files (on the right hand side of the exports map), as is recommended in the TS docs (the output files should be .js(x), not .ts(x)), then extension substitution must take place for the imports to be found.

I think the eslint-import-resolver-typescript is used by enough organizations and people to warrant attention here. E.g. vercel's style-guide.

@andrewbranch
Copy link

andrewbranch commented Apr 2, 2024

Is this just a webpack-bundled app, though? The reason to point imports/exports to output files is if you’re creating output files with tsc and running them in Node.js or another JavaScript runtime. If you’re bundling, or running TypeScript files directly, you can just point everything to the .ts files. Extension substitution only exists because historically, .ts files were not resolvable or runnable by the target host/platform.

@magnusriga
Copy link
Author

magnusriga commented Apr 3, 2024

Is this just a webpack-bundled app, though? The reason to point imports/exports to output files is if you’re creating output files with tsc and running them in Node.js or another JavaScript runtime. If you’re bundling, or running TypeScript files directly, you can just point everything to the .ts files. Extension substitution only exists because historically, .ts files were not resolvable or runnable by the target host/platform.

Indeed @andrewbranch , it is a monorepo with a component library and a few next.js apps, with both .js(x) and .ts(x) modules. Not sure if the components from the component library can be imported by .js(x) modules, if the library's exports points to .ts(x) files.

Also, I was thinking of publishing the component library, and as such it would be nice if any app could consume it.

What do you think?

@andrewbranch
Copy link

andrewbranch commented Apr 4, 2024

A few thoughts.

with both .js(x) and .ts(x) modules

I think we’ve talked about this in another thread. I hadn’t thought much about it before that, but this seems like a recipe for pain when combined with package.json imports/exports wildcards. If the file extension isn’t consistent between all modules that match the wildcard pattern, you have to be ok with specifying the file extension in your import module specifier in the source code.

Not sure if the components from the component library can be imported by .js(x) modules, if the library's exports points to .ts(x) files

They should? Bundlers allow JS to import TS.

I was thinking of publishing the component library, and as such it would be nice if any app could consume it.

I’m still confused about the original request for Webpack to load input TypeScript files when the component library is being intentionally configured to have and resolve to output JavaScript files. You’re doing that so that any app can load it, but you don’t want to load it that way in your own app? Why should your app resolve to the input TypeScript files while others who install your library resolve to the output JavaScript files?

You can have it both ways if you want, using custom conditions:

{
  "name": "component-library",
  "exports": {
    "./Button": {
      "ts-source": "./src/Button.tsx",
      "default": "./dist/Button.js"
    }
  }
}

You could set the ts-source condition in your Webpack config whenever you want it to load the component library’s inputs. You might have it do this all the time, or perhaps only in development mode, so that a production build uses whatever built outputs you’re producing for npm.

File extension substitution is an implementation detail for how TypeScript is able to operate on projects that are written with import paths and package.jsons that are valid for JavaScript runtimes. It’s not a feature that anyone else should copy. I think it would actually undermine TypeScript’s ability to model what it needs to if runtimes and bundlers copied its behavior.

@magnusriga
Copy link
Author

magnusriga commented Apr 8, 2024

@andrewbranch thank you.

I did not intend for myself to import it differently than other consumers. I am ok with all consumers, including myself, to import the components as compiled .js(x) files (i.e. RHS of package.json exports is someName.js(x)).

It seems, however, based on your answer, that it is indeed perfectly OK to use .ts(x) extensions in package.json's exports and imports fields, and that it does not matter whether the importing file (the file with the import statement) is .js(x) or .ts(x) as both can import TypeScript files. This saves having to compile the component library during development, which is nice.

If I understood you correctly, the components can also be published as .tsx files, because the consumers likely all use a bundler.

Lastly, based on your answer, I think eslint-import-resolver-typescript might just have to give up on doing module resolution the exact same way TypeScript does it. To do that, they would have to mimic your extension substitution, by creating their own version of enhanced-resolve.

@alexander-akait
Copy link
Member

Lastly, based on your answer, I think eslint-import-resolver-typescript might just have to give up on doing module resolution the exact same way TypeScript does it. To do that, they would have to mimic your extension substitution, by creating their own version of enhanced-resolve.

I don't mind if we introduce your behavior as an option that will be disabled by default, and describe in the documentation that this is a feature for tooling only and should not be used as part of a normal setup, I understand that duplicating logic and rewriting the same thing adds problems to all of us

@alexander-akait
Copy link
Member

alexander-akait commented Apr 8, 2024

And I'm glad to support other projects that use our library because it allows us to make it more stable and find more problems/bugs

@magnusriga
Copy link
Author

Great, @alexander-akait , sounds like an excellent approach. I am sure all users of that ESLint extension will be thankful for that addition (it is a relatively popular package).

@alexander-akait
Copy link
Member

@magnusriga The only one - I'm afraid that it will be difficult for me to find time to implement this at this moment, there are a lot of tasks for this month, but I will be happy to accept a PR, so PR welcome ⭐

@andrewbranch
Copy link

I’m curious why eslint-import-resolver-typescript, if their goal is to resolve files like tsc, doesn’t just use TypeScript’s module resolution API?

@magnusriga
Copy link
Author

I have no idea, @andrewbranch . @JounQin might know. I just peaked at the source and saw they rely on enhanced-resolve and built support for tsconfig path field on top of it.

@JounQin
Copy link
Member

JounQin commented Apr 9, 2024

We want to resolve paths without relying on heavy typescript package just like using with webpack as described at #355, because we can be using esbuild/swc/babel, etc.

So technically it's duplicate of #355, right?

And I was expecting extensionAlias option would work as we requested in this case.

@alexander-akait
Copy link
Member

The same problem for alias, maybe we need something exportFieldAdditional: ['extensionAlias', 'alias'] (maybe better name)

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