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

Require with non-relative path can pull in unexpected code #294

Closed
robertknight opened this issue Jul 29, 2014 · 4 comments
Closed

Require with non-relative path can pull in unexpected code #294

robertknight opened this issue Jul 29, 2014 · 4 comments
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@robertknight
Copy link

Issue moved from http://typescript.codeplex.com/workitem/2448
If a require statement has a non-relative path (eg. require('mymod') ) then tsc will search all directories from the container of the requiring file all the way up to the file system root for a corresponding source file.

This can result in the require pulling in a completely unexpected source file.

In my scenario I had the following files:

  1. /home/user/myproject/crypto.ts
  2. /home/user/myproject/typings/DefinitelyTyped/node/node.d.ts
  3. /home/user/myproject/lib/test.ts

test.ts uses a '' directive to reference the Node.js typings. The node.d.ts file happens to have an 'import crypto = require('crypto')' statement in it to get Node.js' crypto module.

When tsc was invoked in the 'lib' directory with 'tsc test.ts' and it encountered the node.d.ts file reference, it ended up looking for:

/home/user/myproject/typings/DefinitelyTyped/node/crypto.ts
/home/user/myproject/typings/DefinitelyTyped/crypto.ts
/home/user/myproject/typings/crypto.ts
...
/crypto.ts

Since I happened to have a file with this name in one of these parent directories, tsc ended up trying to compile that code.

This behavior doesn't feel intuitive to me. Node.js' own require statement does something similar but only looks in 'node_modules/' directories at each level, so is less likely to run into the issue.

I'm unsure of what the best behavior would be. Perhaps align the way require() works with absolute paths to match node.js?

@danquirk
Copy link
Member

I've been bitten by this myself in a very similar scenario (using node.d.ts and "events"). Certainly seems like something that could be improved.

We'd need to figure out exactly what the best new behavior would be before we can fix this though.

@danquirk danquirk added Bug and removed Bug labels Jul 29, 2014
@johnpryan
Copy link

I'm curious why the typescript compiler works this way. For example:

// foo.ts
export function run() {
    return 'foo';
}
// myModule/bar.ts
import foo = require('foo');
export function run() {
    return foo.run() + 'bar';
}

In this case the typescript compiler understands where foo is located and outputs:

// myModule/bar.js
var foo = require('foo');                                                

function run() {                                                         
    return foo.run() + 'bar';                                            
}                                                                        
exports.run = run;

This code is not that useful; I'm not aware of any runtimes or javascript bundlers that understand how to resolve the require('foo') statement by looking in the parent directory. Support for node_modules seems to be the correct way to resolve absolute paths.

@basarat
Copy link
Contributor

basarat commented Aug 6, 2014

I'm not aware of any runtimes or javascript bundlers that understand how to resolve the require('foo') statement by looking in the parent directory

Agreed. @danquirk Should be a bug.

@mhegazy mhegazy added Bug and removed Needs Proposal labels Aug 6, 2014
@sophiajt sophiajt added this to the TypeScript 1.2 milestone Aug 11, 2014
@sophiajt sophiajt added Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. and removed Bug A bug in TypeScript labels Sep 8, 2014
@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2015

this is addressed by the node module resolution enhancements in #2338.

@mhegazy mhegazy closed this as completed Dec 1, 2015
@mhegazy mhegazy modified the milestones: TypeScript 1.6, TypeScript 1.8 Dec 1, 2015
@mhegazy mhegazy added Fixed A PR has been merged for this issue and removed Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Dec 1, 2015
@mhegazy mhegazy assigned vladima and unassigned mhegazy Dec 1, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants