Skip to content

Incorrect behavior for moduleResolution classic with relative paths #7051

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

Closed
use-strict opened this issue Feb 12, 2016 · 6 comments · Fixed by #7068
Closed

Incorrect behavior for moduleResolution classic with relative paths #7051

use-strict opened this issue Feb 12, 2016 · 6 comments · Fixed by #7068
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@use-strict
Copy link

When using "moduleResolution" set to "classic", relative paths appear to be resolved in the same way as module paths, by looking at the parent folder. This behavior is incorrect and generates invalid paths.

Example:

// <root>/A.ts
export default class A {
    test() { console.log("hello world"); }
}
// <root>/some/B.ts
import A from "./A"; // <- this should error

let x = new B();
x.test();

This example should not compile. Tested with:
tsc 1.5.3 => doesn't compile (as expected)
tsc 1.6.2, 1.7.5 (moduleResolution = classic) => compiles
tsc 1.6.2, 1.7.5 (moduleResolution = node) => doesn't compile (as expected)

@use-strict
Copy link
Author

Could be related to #5223

@vladima vladima added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Feb 12, 2016
@vladima
Copy link
Contributor

vladima commented Feb 12, 2016

as bizarre as it may sound this is by design - classic resolution strategy always used folder walk to resolve module names. At this point we cannot modify this behavior it as it will be a breaking change since somebody might rely on it.

@vladima vladima closed this as completed Feb 12, 2016
@use-strict
Copy link
Author

@vladima , i'm quite positive that it can't be by design because it behaved as expected in tsc 1.5.3. That is unless 1.6 was a breaking change and both moduleResolution methods were different from 1.5, which seems quite wrong to me. I think you misunderstood and confused this to non-relative paths. Please correct me if I'm wrong.

@RyanCavanaugh RyanCavanaugh reopened this Feb 12, 2016
@RyanCavanaugh RyanCavanaugh removed the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Feb 12, 2016
@vladima
Copy link
Contributor

vladima commented Feb 12, 2016

I've rechecked and yes in 1.5.3 behavior was slightly different:

  • we do discover and include <root>/A.ts into the programm so if this has any contributions into the overall compilation (errors or changes in global scope you'll see them).
  • however during module name resolution in the checker we stopped and don't return anything because the name is relative.

To me this behavior is inconsistent, pinging @mhegazy for his opinion

@mhegazy
Copy link
Contributor

mhegazy commented Feb 12, 2016

looks like a bug to me.

@mhegazy mhegazy added the Bug A bug in TypeScript label Feb 12, 2016
@vladima
Copy link
Contributor

vladima commented Feb 12, 2016

discussed offline: we'll fix the behavior for classic mode to not do folder walk if module name is relative or rooted. As a result it behavior for initial case will be:

  • module resolution error since now we really cannot find ./A.ts
  • no other effect that can originate in ./A.ts for the same reason

@vladima vladima self-assigned this Feb 12, 2016
@vladima vladima added the Fixed A PR has been merged for this issue label Feb 16, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone Feb 23, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants