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

ts loader should resolve relative path's which not starts with './' #29

Closed
stepancar opened this issue Aug 3, 2015 · 13 comments
Closed

Comments

@stepancar
Copy link

Hello @jbrantly !
Thank you for this loader!
I have question
for example we uses import * as mm from '../../MyFolder/myModule';
not will be resolved when call ts-loader, but if we use
'./../../MyFolder/myModule' - all cool.
ts compiler allow use relative paths, which can starts with './' and without it.
What do you think about it?

@jbrantly
Copy link
Member

jbrantly commented Aug 3, 2015

I'm surprised that it works with tsc but not with the loader. I'll have to try it out and see if I can duplicate.

Might be worthwhile to try installing typescript@next and seeing if that fixes it.

@stepancar
Copy link
Author

@jbrantly, i already use latest ntypescript

@jbrantly
Copy link
Member

jbrantly commented Aug 4, 2015

The loader does no module resolving by itself - it delegates all of that to TypeScript. So if it works in TypeScript, it should work in the loader.

I wrote a quick example app and was unable to duplicate. Code like import * as mm from '../../MyFolder/myModule'; worked fine. What is the exact error you're seeing?

The only thing that I can think of is that ts-loader is using an outdated version of TypeScript. If you're using ntypescript, you will need to configure the loader to use that using the compiler option, like:

{ test: /\.ts$/, loader: 'ts-loader?compiler=ntypescript' }

I'd also make sure you're using the latest version of webpack and of ts-loader.

If you continue to see issues, feel free to paste your tsconfig.json and webpack.config.js here so I can try to duplicate further.

@jbrantly
Copy link
Member

jbrantly commented Aug 6, 2015

Were you able to make any progress on this?

@stepancar
Copy link
Author

@jbrantly
I created clean example for this issue. unpack this zip
then npm install (ntypescrip, tsloader)
then run webpack
You will see ts-loader error;
if you change reltive path with './' app will be builded with webpack
Than you!

@jbrantly
Copy link
Member

jbrantly commented Aug 6, 2015

In your example you are using import {Login} from 'components/login';. The module specifier here does indeed need to start with './'. See the CommonJS spec for module identifiers. Basically if you're wanting to reference a relative module you must start the module identifier with a dot, so either './' or '../'. Otherwise the module is considered to be a "global" module, eg: React.

To put it another way, look at this code:

import * as React from 'react';
import {Login} from 'components/login';

When you say 'react' there, you really mean a file in node_modules/react. But when you say 'components/login', you really mean a file relative to this file: './components/login'. TypeScript and webpack have no way to divine what you really mean, so you must explicitly tell them that the second module is relative.

In your original post you mentioned that module identifiers like '../../myModule' weren't working. There is no reason that shouldn't work though because '../' indicates that it is a relative module. Do you have an example of that not working?

@stepancar
Copy link
Author

Thank you for your answer!
Really, i haven't errors with paths like '../../foo'
But i catch ts-loader error when relate to modules like './foo/fooo'. I know that in node module i must start module identifier with './' as you wrote. But why when i use ts compiler i havnt any problemth with paths? Will agree, if sumbody already have a lot of code which uses path like 'foo/foo' and can be compiled without any errors - he shoul rewrite all paths to './foo/foo' for working with ts-loader.
Thank you!

@jbrantly
Copy link
Member

jbrantly commented Aug 6, 2015

You are right. ES6 modules, as near as I can tell, are "implementation-specific" when it comes to resolving module identifiers (meaning they don't necessarily have to follow CommonJS rules). However, webpack (and node) work off those rules and in this case webpack is actually throwing that error. In order words, using "./" is a requirement of webpack (for now at least, they are working on a ES6 compatibility for webpack2).

I'm actually a little surprised that TypeScript resolves it though. Learn something new...

@stepancar
Copy link
Author

I think that you should write about './' path in ts-loader requirements.
Maybe we should create issue for typescript compiler? I created issue for atom-typescript which autocomplete import path without './'
What do you think about it?

@jbrantly
Copy link
Member

jbrantly commented Aug 7, 2015

I've created issue microsoft/TypeScript#4214 for it. It seems to me that TypeScript should throw an error if configured to output CJS (which it is by default when using ts-loader).

@alexdrel
Copy link

alexdrel commented Aug 7, 2015

I want to add that one can configure 'global' module locations using modulesDirectories in webpack config, like this:

resolve: {
 modulesDirectories: [ 'node_modules', 'bower_components', 'src', 'style']
}

We found it pretty convenient (adding src and style folders) as it allows us to make 'local' module loading independent from the .ts file path depth. So instead of require('../lib/mylib.ts') in one file and '../../../lib/mylib.ts' in another we can just require('lib/mylib.ts'). Tracking relative position of styles/assets files is even more annoying.

I was fully prepared to forgo this convenience to achieve typescript@next side compilation (w/o webpack) but was pleasantly surprised to find out that it is working perfectly fine probably because tsconfig.json location is considered a root location for global modules by ts@next .
In my opinion it makes a perfect sense - as typescript is not aware of/using node_modules folder so there is no need to reserve the first class import syntax for it.

@jbrantly
Copy link
Member

jbrantly commented Aug 8, 2015

@alexdrel You're absolutely right, thanks for pointing this out. It highlights the complexities involved. There is a fairly wide gap between what a loader can do (any loader, not just webpack) in terms of configuring module resolution and what TypeScript can currently do. The good news is that they're aware of the issue and there is an effort to fix it.

I still think TypeScript is in error with allowing relative imports without './' or '../' personally. That doesn't really seem to be a "thing" as near as I can tell. Resolving relative to the root is pretty cool though, I'll have to keep that in mind for my own stuff 👍 thanks

@jbrantly
Copy link
Member

The TypeScript folks have confirmed that this is (essentially) a bug in TypeScript in the sense that the ES6 module resolution logic right now is quick and dirty and is changing soon. The new default logic should require './' or '../' for relative modules. Going to close this as the loader itself is working as intended.

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

No branches or pull requests

3 participants