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

Example apps should include file extensions in import statements #8337

Open
GeoffreyBooth opened this issue Jan 4, 2023 · 10 comments
Open

Comments

@GeoffreyBooth
Copy link

GeoffreyBooth commented Jan 4, 2023

Describe the problem

Per @Rich-Harris in nodejs/node#46074 (comment):

. . . allowing modules to import each other without specifying the .js extension was a silly mistake that added untold complexity for no good reason. It makes authored code more ambiguous and less self-explanatory, makes runtimes slower, and adds a complexity tax to tooling that has compounded immeasurably over time. It was a horrible error.

I ran npm create svelte@latest today and chose “SvelteKit demo app,” “JavaScript with JSDoc comments” and looked around at the files, and many had references to extensionless files. For example, src/routes/sverdle/+page.server.js has import { Game } from './game'; (instead of ./game.js) and src/routes/sverdle/game.js has import { words, allowed } from './words.server'; (instead of ./words.server.js).

Describe the proposed solution

Let’s ensure that all the example import statements that refer to individual files (not library exports) include extensions, even for JavaScript files.

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

@Rich-Harris
Copy link
Member

This is an unintended consequence of the fact that those files are generated from typescript source, but touché - we should definitely fix this

@dummdidumm
Copy link
Member

While I agree this was a design mistake in node module resolution, I don't think we should change it right now.

  • people will probably be confused by it because it's not as common
  • even more so if you use typescript, where imports still need to have a .js ending
  • IDE auto imports will do all other file imports without the file ending (except you add a specific configuration, which almost noone uses), so you'll end up with a weird mix

@Rich-Harris
Copy link
Member

people will probably be confused by it because it's not as common

I don't agree with this — people might be surprised, because they're used to doing things wrong, but no-one will be confused by it since it reduces ambiguity.

even more so if you use typescript, where imports still need to have a .js ending

not sure I follow? I'm only suggesting we'd add the .js extension when starting a JavaScript project

IDE auto imports will do all other file imports without the file ending (except you add a specific configuration, which almost noone uses), so you'll end up with a weird mix

The VSCode default is to match existing imports — if you have an existing import foo from './foo' in a given module then bar will auto-import from ./bar, but if you have ./foo.js it will auto-import from ./bar.js. (It's true that if a module doesn't yet have any imports it will default to omitting the extension, which is a bafflingly short-sighted decision from the VSCode maintainers that I've tried in vain to change.) I don't know what other editors do.

@GeoffreyBooth
Copy link
Author

https://www.typescriptlang.org/docs/handbook/esm-node.html recommends including the .js in the import statement within TypeScript files intended for running as ESM in Node. Isn’t the solution here to just update the TypeScript files for the demo app to include .js? That should work for both the TypeScript version and the JavaScript one.

@Rich-Harris
Copy link
Member

The TypeScript files aren't intended for running as ESM in Node though — they're consumed via a bundler rather than transpiled 1:1. It would 'work' for the TypeScript case but I think it's just too weird to have modules explicitly referencing .js files that don't actually exist.

@dummdidumm
Copy link
Member

Same is true for the JavaScript files though, it's all processed by Vite

@Rich-Harris
Copy link
Member

Of course, but the arguments in favour of using file extensions still hold

@gtm-nayan
Copy link
Contributor

FWIW TypeScript is introducing new moduleResolution options for 5.0, microsoft/TypeScript#50152, which would allow using .ts extensions

@Rich-Harris
Copy link
Member

In which case we should definitely switch to using file extensions when 5.0 lands. In the meantime the question is whether 4.x's behaviour is reason enough to keep doing the wrong thing for JavaScript projects in the name of consistency, to which I would argue the answer is definitely 'no'

@Rich-Harris Rich-Harris added this to the soon milestone Feb 1, 2023
@lachlancollins
Copy link
Contributor

Hi, I just wanted to bump this discussion now that TS 5.0 is well and truly out. "moduleResolution": "bundler" allows optional ".js" extensions in imports, as well as optional ".ts" extensions in imports if allowImportingTsExtensions is true. Since both svelte-kit and svelte-package build through Vite rather than tsc, I think this should be compatible with existing projects.

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

6 participants