denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
97.85k stars 5.39k forks source link

Cannot resolve module without extension defined #2506

Closed Drag13 closed 5 years ago

Drag13 commented 5 years ago

Hi guys! First of all let me say big thanks for all the job you already done! Deno looks extremely promising even before release.

Right now I am trying to get a bit more familiar with it and just spotted some kind of issue (I think this is issue but may be I am wrong) when importing my own typescript module.

Let's say we have this code

import { Timer } from "./serivce";
const id = new Timer().start();

It looks OK, but Deno will refuse to run it because it expects to see extension of the file:

Uncaught NotFound: Cannot resolve module "./serivce"

If I will change statement to use extension, as Deno expect, two bad things happens.

The first issue can be partially fixed using //@ts-ignore. But then, I have to add this every time, and that's a bit annoying. And the second issue seems not fixable for me (again, may be just not found the solution)

So my question: Is it possible to make Deno and Ts more friendly and get rid of this //@ts-ignore?

bartlomieju commented 5 years ago

Using explicit .ts extension is required in Deno.

For some reference with VS Code check this sources:

Drag13 commented 5 years ago

Yes, I know that Deno requires implicit extension (I wrote about that), but this cause some problems I've listed before. So is there is there are any plans to harmonize Deno and "common import" to avoid workarounds?

And thank you very much for the provided links!

kitsonk commented 5 years ago

So is there is there are any plans to harmonize Deno and "common import" to avoid workarounds?

What do you mean by "common import". Most browsers require ES modules to include their extension. Node.js has also gone down that path for their support for ES modules.

Also see #2329. We tried it, it made things a lot more complicated, and so we removed it. It is very intentional at the moment to keep it out.

Drag13 commented 5 years ago

@kitsonk, please don't get me wrong. I am not trying to blame or force someone. I am just trying to understand current concept and your answer with reference is perfect in this meaning. Thanks, I'll close the issue.

ry commented 5 years ago

Maybe we can have a better error message when extensions are omitted? Or maybe we should add some text to the manual explaining the situation and our plans with import specifier extensions.

Drag13 commented 5 years ago

From my point of view, main problem is that those who use VsCode (like me) get used auto import. And when you are trying Deno - it fails. Adding extension explicitly doesn't help much because than it start failing in another way. So, main question appears:

If I can't use my everyday tool, maybe Deno is not ready yet?

At this moment some explanation (and links posted above especially) will be extremely helpful, because in real life Deno is awesome! But mentioned scenario may stop many and many people from using it. Another point is that, if I already have project - I can't start using Deno, because I have to update imports manually. And after that I can't switch back, because I need to update imports again. This is also kind a stopper.

So, message with explanation and possible solution will be nice (and much better than current). But altering importing mechanism may be a good option also.

Sorry for such long text.

ry commented 5 years ago

Deno is certainly not ready: https://deno.land/manual.html#disclaimer

I think this is the most mature vscode plugin - https://github.com/justjavac/vscode-deno - does that not solve the problem?

But mentioned scenario may stop many and many people from using it

Yep. We have certain design goals that we don't want to compromise for short-term fixes. It's important to get our core infrastructure working as intended ... making a smooth transition for people with existing software is something that will be considered later once we don't have critical bugs. Some things just take time. I appreciate you taking the effort to communicate where it went wrong for you - that's important information for us to know.

Drag13 commented 5 years ago

Deno is certainly not ready

Not ready for Prod? Yes, certainly. Not ready for pet projects, every day scripts, experiments, PoC? I think it's good point to start with Deno.

Extension solve problem partially - I still can't transfer code from Node to Deno and vice versa. But it's more-less OK for me, so I can say that my problem solved.

jpike88 commented 5 years ago

Ok this to me seems a little odd.

What's the point of ingesting TS files if there's a fundamental, breaking difference in syntax? Are they still 'TypeScript' files at that point? Every person who wants to use Deno will have to do some weird tweak with their codebase and dev tooling that deviates from standard VSCode configuration, breaking existing compatibility with TS (Which means an independent type check on CI for example wouldn't work)...

It would be a tiny amount of code that attempts to resolve a path if there's no extension, at a guess.

if (noExtension) {
    try {
        return attemptTs() || attemptJs();
    } catch (error) {
        throw new Error('No module found here.')
    }
}

This will open up Deno properly to the existing TS community. Deno will benefit from increased usage, attracting more contributors, and at any point you can deprecate that functionality with little effort?

Yep. We have certain design goals that we don't want to compromise for short-term fixes

I don't think this would be a short-term fix, rather a long-term strategy which takes into account high uncertainty in it being resolved anytime soon. Looks like the TS team is not even considering modifying how they resolve paths at this point.

https://github.com/Microsoft/TypeScript/issues/27481

kitsonk commented 5 years ago

What's the point of ingesting TS files if there's a fundamental, breaking difference in syntax?

The module specifier is not part of the syntax of the language, and isn't actually specified anywhere. TypeScript will eventually remove the warning or provide a flag that allows it. The world is going the ways of fully specified path names because the extension resolution "magic" has caused so many problems.

Looks like the TS team is not even considering modifying how they resolve paths at this point.

I disagree, both Ry and I have had some conversations with the team. It is something they are going to have to tackle with the way Node.js has gone as well. We will continue to work with them as time goes on. Doesn't mean we always agree though, but I can assure you they haven't closed the door.

jpike88 commented 5 years ago

TypeScript will eventually remove the warning or provide a flag that allows it.

We will continue to work with them as time goes on

I mean, I get the sentiment here.

But if deno relies on TS projects, shouldn't it be tracking current TS capabilities? Unless you guys are willing to wait it out... but I would have figured getting it more exposure sooner is a better move? This would be a backwards compatibility thing that you could deprecate when it happens.

But when would that be? End of year? Maybe later?

kitsonk commented 5 years ago

We aren't looking for exposure or more than we already have. The Deno repo has 35k likes. It is already at times like riding a bull. Not that peoples contributions are invaluable, but on this particular point we tried it (#857 and look at who raised the issue 😬 ). Especially when it came to remote modules, it was a horrible waste "guessing" at what the extension might be. Especially with the goal of having both JS and TS supported, it means extension-less is 2-3 full return network requests for each module before you can proceed with the analysis of the dependency graph. So when we saw the reality of our decision, we reverted and realised that "magic" module resolution is actually really bad.

I suspect the reality of getting tsc to support fully qualified extensions and a literal resolution mode is contributing something back to the TypeScript. I haven't taken a look in a while of what it would take to deliver a "literal" module resolution mode, but maybe I should again.

kitsonk commented 5 years ago

Also, I should mention that I think in the long term, to boost adoption, a CDN that would fetch and cache other modules and rewrite them to be fully qualified would be a possibility. Things like Pika CDN already provide ESM bundles transpiled and don't include polyfills for modern browsers. We wouldn't be talking about something too different here.

patomation commented 4 years ago

@kitsonk Just allow magic extension-less imports of ts files end require extensions for everything else. Problem solved? deno primarily consumes TypeScript right? For example, why not assume if there is no extension look for a file with the same name with the ts extension. If one is not found, throw an error. Do not use magic imports for .js files. Make those explicit. Thoughts?

kitsonk commented 4 years ago

Just allow magic extension-less imports

Very much against the design goals of Deno.

deno primarily consumes TypeScript right?

Depends, there is a lot of folks that run just JS under Deno. I don't think it is good to assume one or the other.

jerrygreen commented 4 years ago

That's weird that some popular packages like Ramda have extension-less imports, check it out:

https://deno.land/x/ramda@v0.27.1/difference.js

Is it a mistake of Ramda, or something's changed, or it's supposed to work with js extension and only ts in unsupported?

I reported this on Ramda repository too:

https://github.com/ramda/ramda/issues/3091

Josema commented 3 years ago

Is there a reason for this behaviour? Is a bit annoying when you come from node.js

kitsonk commented 3 years ago

The reason for this behaviour is a) it aligns to the behaviour of web browsers, b) it is unambiguous, allowing Deno to easily support foo.ts and foo.js without problems or issues. Having "magical" resolution is one of the big mistakes of Node.js (and caused no end of pain of the transition to ESM), and Node.js is even headed that way as far as explicit extensions.

nuts-n-bits commented 3 years ago

Looks like the TS team is not even considering modifying how they resolve paths at this point.

I disagree, both Ry and I have had some conversations with the team. It is something they are going to have to tackle with the way Node.js has gone as well. We will continue to work with them as time goes on. Doesn't mean we always agree though, but I can assure you they haven't closed the door.

Well, this. They a couple months ago made it clear TS will not consider even behind a flag to touch the module specifier in any way. Does this mean that if Deno were to continue to work there have to be a fork of tsc?

kitsonk commented 3 years ago

Does this mean that if Deno were to continue to work there have to be a fork of tsc?

Deno has been working just fine with an un-forked version of tsc for a couple years. 🤷

nuts-n-bits commented 3 years ago

Deno has been working just fine with an un-forked version of tsc for a couple years. 🤷

I'm curious, how are you making it so that the non-forked tsc is gladly eating up import specifiers that end in ".ts"? I'm working on a library recently and I wish to publish it targeting both node and deno. Over the past several days however it has become clear to me tsc is not the tool for the job.

al6x commented 3 years ago

Just wondering if there any news? As currently it's impossible to share files between Deno and React/Svelte. As all major Front-End Packages using Node.JS convention and don't allow *.ts.

I managed to make it work by writing bash scripts that copy shared files and re-writte it by deleting ".ts" postfixes, but there should be a better way.

tmladek commented 3 years ago

As currently it's impossible to share files between Deno and React/Svelte.

Or Vue. I actually ended up at a similar "solution" - a Makefile that copies the shared files over, a regex that removes the extensions, and an appropriate .gitignore... Obviously this is not ideal.

I don't mean to imply it's the job of Deno to sort this out (for that matter, I'm personally in favor of fully qualified paths as well). I just wonder if there's a reasonable workaround.

micheal-hill commented 3 years ago

Deno has been working just fine with an un-forked version of tsc for a couple years. shrug

I'm curious, how are you making it so that the non-forked tsc is gladly eating up import specifiers that end in ".ts"? I'm working on a library recently and I wish to publish it targeting both node and deno. Over the past several days however it has become clear to me tsc is not the tool for the job.

@nuts-n-bits After much head banging, I've found this which seems to be how Deno is making tsc allow .ts files. In short; it doesn't have tsc allow .ts files, it just ignores the errors that the compiler spits out.

@kitsonk maybe this is worth documenting somewhere? Trying to find this buried in the repo is rather difficult, since most of the repo is rust and searching for any reference to tsconfig or tsc shows rust files. Which, I'm assuming the majority of the consumers of deno are not fluent in - I know I'm not.


In my case, I was also hoping to have a library that's used by both Deno and NodeJS. Unfortunately this doesn't seem possible in any scalable way without transforming the source in one way or another; so I'm going to stick with NodeJS wherever possible.

In summary, it seems the problem with cross-compat between Deno and NodeJS is:

cfjello commented 2 years ago

I just did a quick port of the very useful package XRegExp source files to make them work under Deno and for package it was not that difficult:

Here are some of my very own opinions, having used Deno for a long time:

What puzzles me is that the original XRegExp source files works fine with babel, ES6, browserify, node, dtslint, eslint and typescript, basically everything with the exception of Deno! (and I think the exclamation mark is warranted).

It seems that the so-called 'magic extension-less imports' is the de facto norm and not at all magic, but rather, it falls into the category of common sense. Personally, I do not buy into the purist argument at all - I prefer productivity.

A quick look at Google trends, comparing Nodejs and Deno clearly show the Deno adoption problem, which is mainly down to these sorts of time consuming incompatibilities.

So I urge you to provide a Deno flag to allow for detection of .js and .ts extensions, this could easily be a very shallow implementation, or as an alternative, provide us developers with a way to proxy the 'import' command, so that we can fix the very real problem.

bartlomieju commented 2 years ago

@cfjello have you tried running XRegExp package without changes using "compat mode"? https://deno.land/manual@v1.17.3/npm_nodejs/compatibility_mode

lucacasonato commented 2 years ago

@cfjello Deno does not arbitrarily require file extensions just because we think it looks snazzy. We do because "magic resolving" requires file system probing, and that is fundamentally incompatible with remote (http) imports.

File system probing can be done on remote specifiers, because: a) probing 10s or 100s of possible specifiers is very very expensive in terms of latency, and would significantly slow down module acquiry in Deno. This is thus infeasible. b) the alternative to probing, directory listing, is not possible on remote servers. There is no standard way to get the directory listing of a remote HTTP server.

Node also does not support "magic resolution" on relative specifiers in ESM. To use the native ESM in Node, your relative specifiers need to include .js extensions. See https://nodejs.org/dist/latest-v17.x/docs/api/esm.html#mandatory-file-extensions. Browsers also require file extension on module specifiers.

So in conclusion: all engines that support loading ESM natively (web browsers, Node, and Deno) require file extensions on ESM specifiers. As such I don't think it is fair to say that Deno is the odd one out here. The tooling ecosystem in Node has many non standard ESM implementations that will need to converge on the standard that Node's ESM implementation has set.

cfjello commented 2 years ago

@lucacasonato This i why I suggest a shallow implementation, where the cost is incurred only by those who explicitly and knowingly sets the flag and I do not see why you would ever implement such a feature when importing from a remote server using HHTP - what would be the the use case? @bartlomieju Thank for the tip. I will try it.

lucacasonato commented 2 years ago

@cfjello Did you read the rest of the issue? Node also does not support "magic resolution" for relative and absolute specifiers in ESM. Why would we want to support this now, if even Node doesn't support it itself?

The flag you are looking for is --compat. It will soon allow you to load local CJS code from ESM code.

cfjello commented 2 years ago

@lucacasonato Thanks - I will try it and report back

cfjello commented 2 years ago

@lucacasonato @bartlomieju

I just did the test based on:

npm i xregexp deno run --allow-all --unstable --compat importTest.js

importTest.js file is:

// @deno-types='./node_modules/xregexp/types/index.d.ts' import XRegExp from './node_modules/xregexp/src/index.js'

The source index.js file looks like this:

import XRegExp from './xregexp';

import build from './addons/build'; import matchRecursive from './addons/matchrecursive'; import unicodeBase from './addons/unicode-base'; import unicodeCategories from './addons/unicode-categories'; import unicodeProperties from './addons/unicode-properties'; import unicodeScripts from './addons/unicode-scripts';

build(XRegExp); matchRecursive(XRegExp); unicodeBase(XRegExp); unicodeCategories(XRegExp); unicodeProperties(XRegExp); unicodeScripts(XRegExp);

export default XRegExp;

And the package.json file includes the line:

module: "./src/index.js"

I tried different combinations, renaming the test file to importTest.mjs or setting "type": "module" in my own package.json, or fetching the index.js from /src/index.js instead of /lib. but I get various missing module, no default export errors.

I am not an expert in ESM, but as stated earlier, the code is pretty much vanilla JavaScript and I know what to change to make it work. in the Deno environment.

I do not know the specifics of how Node do or do not support 'magic resolution of imports' for ESM and/or vanilla JavaScript, but keep in mind, that this is working code using 'import' within the Nodejs environment with 7,890,538 million weekly downloads.

ryan-s-wilson commented 1 year ago

I want to use deno on a code base that has no intentions of supporting deno (specifically I want to use the language server) @kitsonk do you have an idea where it would be best to make a change so I could have a fork that supports existing statements such as import Button = require("Everlaw/UI/Button");

I really want deno to work as our code base is pushing ts-ls to the point of non functionality and deno seems far more performant. However the lack of support imports without extensions is currently a deal breaker.

I have looked around for a couple of hours and its not obvious how I could hack in that functionality for my personal use.

neilsarkar commented 1 year ago

Hi there, makes sense that allowing "magic" imports opens up a whole can of worms when trying to auto detect, especially with remote urls.

I'm using deno_core in a rust app and am able to default to MediaType::Typescript if there's no extension on a local file, and this works well for my use case. Perhaps the user could opt-in to parsing any extensionless file as typescript if all of their "magic" imports are ts?