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

import statement broken from migration 1.0.3 to 1.1 or higher #954

Closed
epinxteren opened this issue Oct 24, 2014 · 5 comments
Closed

import statement broken from migration 1.0.3 to 1.1 or higher #954

epinxteren opened this issue Oct 24, 2014 · 5 comments
Assignees
Labels
Bug A bug in TypeScript Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it

Comments

@epinxteren
Copy link

I have searched the github project for any release changes bud my import statements are broken from a code migration of 1.0.3 to version 1.1 or higher. (http://stackoverflow.com/questions/26543594/import-statement-broken-from-migration-1-0-3-to-1-1-or-higher)

Directory structure:

  • BatteryIncluded/TextWriter.ts
  • BatteryIncluded/Registry.ts

Code Registry.ts:

import TextWriter = require("BatteryIncluded/TextWriter");

class Registry <ValueType> {
   // etc...
}

export = Registry;

Code TextWriter.ts:

class TextWriter {
   // etc..
}

export = TextWriter;

In typescript version 1.0.3 the error would not occur. But in version 1.1 or higher the error is:

"C:/Program Files/nodejs/node.exe" C:\PROJECT_GIT\TypeScript\built\local\tsc.js --sourcemap --    target ES5 --module AMD Registry.ts
Registry.ts(2,29): error TS2307: Cannot find external module 'BatteryIncluded/TextWriter'.

Using typescript in the form of:

node C:\PROJECT_GIT\TypeScript\built\local\tsc.js --sourcemap --target ES5 --module AMD

To fix this problem i can use the import statement:

import TextWriter = require("./TextWriter");

This is not desired because all deeply nested files i need to do something like this:

import TextWriter = require("../../../TextWriter");

Before could just use:

import TextWriter = require("BatteryIncluded/TextWriter");

I have searched on TypeScript GitHub repro for release changes that could imply this error, but couldn't find any.

@jednano
Copy link
Contributor

jednano commented Oct 24, 2014

I've been having similar issues with TS 1.1, which is why I'm still on 1.0 for now. I haven't delved into it as much as @epinxteren here, but I remember my errors were similar.

@danquirk
Copy link
Member

The behavior does appear to have changed here and broken things. Could you describe your build setup a little more? How is your module loading system finding TextWriter with the above reference? Do you have other build steps that are moving files around? I find the current behavior far more intuitive, the import in Registry.ts is clearly stating an intention to look inside a folder BatteryIncluded inside the folder that Registry.ts itself is in. I'm not sure how something like node would actually make this work at runtime. That said, we've had persistent issues in this area and we don't want to break old code so we need to figure out a holistic solution (ex #247).

@danquirk danquirk added the Bug A bug in TypeScript label Oct 24, 2014
@epinxteren
Copy link
Author

I'm using AMD with requirejs. I'm not moving any files around and not using Node to execute the files.

Document structure path is: batteryincludedphonongui/js/BatteryIncluded

Requirejs config looks like this:

requirejs.config({
    baseUrl: "/bundles/batteryincludedphonongui/js/",

    paths: {
        // etc
    } 
});

So at runtime the import statement: would work perfectly fine:
import TextWriter = require("BatteryIncluded/TextWriter");

Js version:
define(["require", "exports", "BatteryIncluded/TextWriter"], function(require, exports, TextWriter) {

RequireJs would resolve the file as:
/bundles/batteryincludedphonongui/js/BatteryIncluded/TextWriter.js

Of course i'm minifying and combining files for production use, but this is not relevant.

The cool things is TypeScript also finds the definition of BatteryIncluded/TextWriter completely different path like:

  • js/BatteryIncluded/TextWriter.ts
  • js/App/LogBundle/Controller/LogController.ts

And it works in 1.0:

LogController.ts

import TextWriter = require("BatteryIncluded/TextWriter");

class LogController {

/// etc..

The 1.0 version could find any import statement inside those path's, what is very handy.

@sophiajt sophiajt added this to the TypeScript 2.0 milestone Oct 28, 2014
@nitram509
Copy link

Same problem when compiling to CommonJS modules.
Using this workaround at the moment :-/

Workaround, when developing Node.JS (commonjs) modulus

/// <reference path="../../typings/node/node.d.ts" />

declare var require; // TODO: bad hack to make TSC compile, possible reason https://github.com/Microsoft/TypeScript/issues/954
var crypto = require('crypto');

// sample usage
var mac = crypto.createHmac('sha256', key);

Any chances to fix this soon?

@mhegazy
Copy link
Contributor

mhegazy commented Apr 22, 2015

The underlying issue should be handled by #2338, which should allow for basepath, as well as an explicit path mapping to avoid relying on internal compiler details like what happened in this case.

closing in favor of the general solution in #2338.

@mhegazy mhegazy closed this as completed Apr 22, 2015
@mhegazy mhegazy added the Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it label Apr 22, 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
Bug A bug in TypeScript Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it
Projects
None yet
Development

No branches or pull requests

6 participants