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

A compiler option to enforce file extensions. #42813

Closed
5 tasks done
trusktr opened this issue Feb 16, 2021 · 19 comments
Closed
5 tasks done

A compiler option to enforce file extensions. #42813

trusktr opened this issue Feb 16, 2021 · 19 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@trusktr
Copy link

trusktr commented Feb 16, 2021

Suggestion

A compiler option like requireExtensions would be great.

🔍 Search Terms

typescript compiler require import extensions

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

A compiler option like requireExtensions would enforce .js extensions to make modules ESM-compatible without additional tooling, imports maps, or package.json exports fields.

📃 Motivating Example

ESM ecosystem (Node, Webpack 5+, etc) follows the Node ESM standards. Tools like Webpack will throw a compile error on non-confoming ES Modules (when they don't conform to Node ESM spec), and Webpack full understands the package.json exports field when resolving modules for example.

💻 Use Cases

This would help to enforce writing code that is more likely to work well in strict Node ESM environments.

In Webpack 5, importing TypeScript output code that does no include extensions (although TS gives no error) results in a compile error for packages with "type": "module" in their package.json during Webpack's resolution (it behaves identical to Node ESM).


With requireExtensions set to false, the following code would be valid:

import {foo} from './foo'
import {bar} from 'some-lib'
import {baz} from 'some-lib/baz'
import {lorem} from '@org/some-lib/lorem'

With requireExtensions set to true, the following code would be valid only if moduleResolution is also set to "node":

import {foo} from './foo.js'
import {bar} from 'some-lib'
import {baz} from 'some-lib/baz.js'
import {lorem} from '@org/some-lib/lorem.js'

otherwise with a different value of moduleResolution it would probably need to be

import {foo} from './foo.js'
import {bar} from 'some-lib.js'
import {baz} from 'some-lib/baz.js'
import {lorem} from '@org/some-lib/lorem.js'
@trusktr trusktr changed the title Add a compiler option to enforce file extensions. A compiler option to enforce file extensions. Feb 16, 2021
@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Feb 16, 2021
@pwhissell
Copy link

Because nodejs requires file extensions for relative imports,
This would be very helpful in modern nodejs projects trying to use ESM modules.
This will be more and more recurring upon TC39 module proposals being finalized (Top-level await, JSON modules, Module blocks, etc.)

@cherryblossom000
Copy link
Contributor

cherryblossom000 commented Jun 10, 2021

In the meantime, you could use eslint-plugin-import's extensions rule with the "always" option to enforce extensions.

It would be nice if TypeScript could enforce this though. TypeScript should be able to detect simple runtime errors like not including a file extension in an import.

@rossng
Copy link

rossng commented Jul 2, 2021

I think this is becoming increasingly important. There has been a significant move towards ESM-only packages in the last few months.

For example, I am using the d3-format package (2.6 million weekly downloads). The latest version has dropped CommonJS support entirely. After updating, I immediately ran into the fact that you can't import ES Modules from CommonJS. I flipped my tsconfig.json over to "module": "es2015", thinking that would solve it.

How naive I was! My project built fine, only to fall over immediately when launched in Node. The problem? My ESM import paths absolutely must have .js extensions to work.

After reading through the notorious #16577, I had the same thought as @trusktr. I'll tolerate spending a few hours to rewrite all my import paths with a .js extension. But it is absolutely critical for me that the compiler rejects invalid imports, and that tsserver/Intellisense suggests the correct ones.

Yes, you can set a config flag in VSCode, and add an ESLint rule. But not everyone uses VSCode, and some people will manage to avoid running ESLint. And that means that a catastrophic runtime exception will probably sneak into a production deployment at some point.

Adding this flag or a new moduleResolution option would fix it. Then there will be a simple and clear solution for the tens of thousands of developers who inevitably try to update a package in the next few months and smack head-first into this same problem.

@Hazmi35
Copy link

Hazmi35 commented Jul 19, 2021

node-fetch just moved to pure ESM on v3.0.0-beta.10 too

@lizthegrey
Copy link

I am likewise here from https://blog.sindresorhus.com/get-ready-for-esm-aa53530b3f77#23d5 because p-limit package did the big breaking upgrade.

ryanjwessel added a commit to ryanjwessel/pear-cli that referenced this issue Sep 6, 2021
I'm not crazy about this change, as I liked it when it inferred the extension automatically.

Also, it feels weird to provide a .js file extension when I'm writing TypeScript files.

However, this seems to be necessary for pure-ESModule packages, and after installing `markdown-table` I need to make these changes.

[1]:https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c#faq
[2]:microsoft/TypeScript#42813
[3]:https://www.typescriptlang.org/docs/handbook/module-resolution.html#classic
[4]:https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules
@trusktr
Copy link
Author

trusktr commented Sep 13, 2021

Random tip: if you happen to be using Webpack 5 or higher, it will automatically complain if you forgot a .js extension. It's not a TS-specific solution, but at least it will help if you're already using Webpack.

This leads to a temporary workaround: you can test your code by running Webpack on it, even if you just ignore the result, and you'll get errors on missing extensions.

@andrewbranch
Copy link
Member

This should probably be closed by #44501, in my opinion. It doesn’t make sense to say “make me write file extensions” outside the broader context of a module resolution strategy where file extensions are required, and --moduleResolution node12 is the first of those strategies that we’ll support. More exist; we still need a resolution mode well-suited for browser-flavored ESM, but new module resolution strategies are wholesale features that should be tracked as such.

@rossng
Copy link

rossng commented Sep 14, 2021

Does #44501 enforce the presence of file extensions or does it magically transpile to the correct import statement? I couldn't figure it out from skimming the PR.

@andrewbranch
Copy link
Member

It enforces file extensions. TS will never rewrite an import declaration’s module specifier under any mode, and in Node’s ESM implementation, file extensions are required, therefore we must make you write them.

@rossng
Copy link

rossng commented Sep 15, 2021

Awesome. Sounds like this will work in browsers and support export maps too - I am really looking forward to it.

@andrewbranch
Copy link
Member

Closing since #44501 is in now. Use --moduleResolution node12 or --moduleResolution nodenext to enforce specifying .js on relative imports.

@richardkazuomiller
Copy link

Has anyone successfully gotten ESLint to enforce the .js extension on .ts imports? I've tried using and it just says Missing file extension "ts" for "../utils/constants.js". I don't know what the configuration is for "yes, there is not a .js file there, and if I'm using ts-node or a bundler, there never will be, but please treat all .ts files as if they will be converted in to .js files sometime in the future."

@fedulovivan
Copy link

Using --moduleResolution with esnext still does solve the issue. Since awkward *js suffix is still needed when you importing TS file which is illogical. Just a a workaround for workaround..

@KonnorRogers
Copy link

KonnorRogers commented Dec 6, 2023

Closing since #44501 is in now. Use --moduleResolution node12 or --moduleResolution nodenext to enforce specifying .js on relative imports.

This doesn't quite fix the problem though.

If I have TSConfig for a browser environment, I would also need to set the target to be NodeNext. Which means when i import a package that dual ships browser and node types, itll grab imports for the node export condition, and not the browser export condition.

It feels like there would need to be a way to opt into NodeNext module resolution, but still be able to use module: "ESNext"

@andrewbranch
Copy link
Member

Can you clarify your setup a little bit? What tool is resolving node_modules packages to the browser conditional export in this scenario? We typically see this done by bundlers in the context of a whole bundled app, which would mean you don’t need to include file extensions on relative import paths. Are you shipping unbundled ES modules to the browser? Are you using import maps to reference dependencies? Is a tool generating those import maps? Give me as many details as you possibly can, please 😃

@KonnorRogers
Copy link

@andrewbranch Sure! The setup is for a component library called Shoelace. https://github.com/shoelace-style/shoelace/

We have a library we import here called qr-creator https://github.com/nimiq/qr-creator/tree/master

we import it here:

https://github.com/shoelace-style/shoelace/blob/a3496f920fa83652d9a90fb99e734881b66a20ab/src/components/qr-code/qr-code.component.ts#L5

and then in a PR I have with NodeNext, it requires this hacky workaround because it think its a Node import and not a "default" import.

https://github.com/shoelace-style/shoelace/blob/a3496f920fa83652d9a90fb99e734881b66a20ab/src/components/qr-code/qr-code.component.ts#L54-L55

I was wrong about the "browser" export condition. The package doesnt appear to have a browser export conditional when examining the package as I initially thought, instead, it appears it to behave like a module.exports for some reason I'm not sure of.

We are shipping unbundled ESModules to NPM for users to consume.
No importmaps.

Basically, when I change our TSConfig from the following:

{
    "target": "es2017",
    "module": "es2020",
    "moduleResolution": "Node",
}

to the following:

{
    "target": "es2017",
    "module": "NodeNext",
    "moduleResolution": "NodeNext",
}

That QRCreator.render fails with: typescript: Property 'render' does not exist on type 'typeof import("/Users/konnor/projects/shoelace-worktree/next/node_modules/qr-creator/types/qr-creator")'.

and our ESLint ends up throwing 1000+ error for things like this:

  128:23  error  Unsafe member access .shadowRoot on an `any` value                                  @typescript-eslint/no-unsafe-member-access

Despite no errors with the previous setup.

Let me know if there's anything else I can do to help clarify.

@andrewbranch
Copy link
Member

andrewbranch commented Dec 6, 2023

What’s happening is that the qr-creator library is providing an ES module that TypeScript thinks is a CJS module, because Node.js would interpret it as a CJS module and crash if it tried to load that file. I think this is a subtle bug in qr-creator. Even though their code is DOM-dependent, they’re distributing their code via npm, which means they expect their consumers to have it in node_modules, so they would do well to conform to Node.js packaging conventions. Additionally, anyone who might want to import qr-creator into Node.js with jsdom for testing purposes is currently completely unable to do so. They should rename qr-creator.es6.min.js to qr-creator.es6.min.mjs, rename qr-creator.d.ts to qr-creator.d.mts, and set up package.json "exports" instead of relying on the "module" field.

The reason we suggest using nodenext for libraries is that if you can make something work in Node.js ESM, it pretty much always works in bundlers and it may work in the browser if import maps are set up appropriately. Here, you’ve found something that will definitely not work in Node.js ESM, even though it will work in the other two scenarios. As a frontend library, you might not care about working in Node.js, but then again, maybe you do—this TypeScript error is giving you a clue that if you import qr-creator, nobody will be able to import your library in Jest, for example, because you have a dependency that will instantly crash Node.js.

@KonnorRogers
Copy link

🤔 okay. That makes sense. I'm not sure what to make of my eslint errors, but that seems more easily fixed. It may just be a config issue with the typescript-eslint-parser.

I'll see if I can make a PR to the qr-code repository.

As for crashing node, there is a case for Jest, as well as Node-based SSR, so will be something we need to address. Thank you so much for your time and help.

@andrewbranch
Copy link
Member

I’m happy to help advise on the qr-creator PR. Good luck though, because its last publish was 4 years ago 😐

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests