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

Supporting extension-less module resolution #857

Closed
kitsonk opened this issue Sep 28, 2018 · 2 comments · Fixed by #859
Closed

Supporting extension-less module resolution #857

kitsonk opened this issue Sep 28, 2018 · 2 comments · Fixed by #859

Comments

@kitsonk
Copy link
Contributor

kitsonk commented Sep 28, 2018

Supporting extension-less module resolution should be considered for Deno. It would help adoption, specifically because it would allow authoring of local code in editors with full "out of the box" TypeScript support.

Resolution

This would likely require Deno to attempt to resolve modules in a specific order, applying the extensions. For example:

  • Module Specifier: ./foo/bar
  • Try to resolve: referencing module path + /foo/bar.ts
  • Then: referencing module path + /foo/bar.js

And:

  • Module Specifier: https://example.com/foo/bar
  • Try to resolve: https://example.com/foo/bar.ts
  • Then: https://example.com/foo/bar.js

This could become very costly for remote modules.

Other considerations

Currently Deno doesn't support any sort of "magical" resolution of things like ./foo/bar resolving to ./foo/bar/index.ts and IIRC was part of Ryan's "10 things I did wrong" list. On the other hand, TypeScript, when using in an editor, would follow a module resolution like that. It also has a few other "magical" resolution in line with NodeJS, like namespace resolutions which Deno does not support.

A lot of userland code likely could be already transpiled TypeScript with a .d.ts file and a .js file. It might be worth considering if Deno would add the feature to then type check that code before running it.

Related to #852

@ry
Copy link
Member

ry commented Sep 28, 2018

Yes I ageee - we can support it to help adoption - but I think the code/docs should be heavily commented that it’s meant as a stop gap solution.
I guess where this would happen is in src/deno_dir.rs during resolve_module().
A patch should introduce unit tests in deno_dir.rs and compiler_test.ts

@guybedford
Copy link
Contributor

Note that Node.js itself is currently in the process of considering if this should be removed entirely. There have also been issues with ".mjs" and ".js" extensions both being checked as well causing instance issues with libraries such as GraphQL which seems to also be leading in the direction of remaining explicit.

For what it is worth, my opinion currently is that we should mandate extensions for modules, as a resolver that doesn't touch the file system is a fast resolver.

@ry ry closed this as completed in #859 Oct 2, 2018
ry pushed a commit that referenced this issue Oct 2, 2018
hardfist pushed a commit to hardfist/deno that referenced this issue Aug 7, 2024
Bumped versions for 0.301.0

Co-authored-by: bartlomieju <bartlomieju@users.noreply.github.com>
Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
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

Successfully merging a pull request may close this issue.

3 participants