Multiple local symlinks wrongly leading to TS2345 #6365
Comments
|
Nice write up @ahom. I ran into this exact same problem today but was unable to figure out what exactly was going on. +1 for a fix for this please. |
|
one thing to note, fs.realPath is significantly slow.. see nodejs/node#2680. until this issue is fixed, i do not think we can use fs.realPath. |
|
@mhegazy I'm not sure I get this right, in the linked ticket they point out that resolving ~1000 symlinks would take about 45ms. I understand it is orders of magnitude slower than the C call but is it really impacting that much the big picture? I quickly timed the compilation of my project with and without the call to fs.realpath (without the call it fails midway but it was just to see the timings) and 45ms seems very negligible when looking at the ~1.4 sec it takes to compile. (it only needed to resolve 10 symlinks in my case anyway because it is only called at resolution time of a new external module)
|
|
We will have to do it on reading any files. i do not see why a module is a special case. the impact of small projects may not be big but would be observable on larger solutions. |
|
I'm not sure if this is related, but I'm seeing issues on Windows when trying to extend a class defined in a package that is in a symlinked folder in (I replaced my actual project name with In this example,
The interesting thing is that the file actually exists at that location even though the error says that it doesn't. If I copy the entire folder contents to
Update: Explicitly importing the |
|
@andreialecu I have the same issue, also involving webpack and tls-loader. The file exists, but is located in a symlinked folder. |
|
@sebastianhoitz I was able to get it to work. The trick is to ensure all other dependencies of the imported class are also explicitly imported in your main project, otherwise you will get that error I pasted above. Until this is fixed, I can live with this workaround. Also, make sure your resolve: {
fallback: [path.join(__dirname, 'node_modules')]
},
resolveLoader: {
fallback: [path.join(__dirname, 'node_modules')]
},Otherwise you may run into other issues with symlinked files. |
|
@andreialecu @sebastianhoitz I also got that issue when using
At some point, it leads to some issues because of a cache (of filename -> content) they are using in which they can't find the "unresolved" symlinked path because it is registered (in the cache) with the resolved path. I think that this specific issue should be opened on |
|
@ahom @mhegazy I was able to work around the issue for me without using @ahom 's fix. Now that I understand what is going on I actually think the solution I landed on will be better in the long term for my project. The error I was running into (and it looks like @ahom is running into as well) is at its core an issue with Type Compatibility. My solution was to make sure that all of my exported apis take in interfaces instead of classes. This works because interface type compatibility checks are a lot looser then class compatibility checks. For me private and protected members kept causing problems.
@ahom Unless you intend your node modules to always be paired together, in my opinion it is better to change to this style instead of relying on the Typescript compiler to resolve symlinks for node modules. This is because if you ever upload your node modules independently to npm (or another binary repository) then your modules are likely going to have direct dependencies on the versions of what your other modules import. Not to mention your compile will break at that point because the Typescript compiler will run into the same problem as it is currently (resolves class definitions to different files since there would be no symlinking). Anyway sorry for the unsolicited advice. This is what ended up working for me, let me know if it does not make sense. |
|
@greyepoxy Thanks for sharing your solution! I understand that it can be worked around but I think this is a completely valid usecase that Typescript should be able to handle. When you setup a new project, you want to iterate quickly even if it is spread over multiple libraries and an interface is not always desirable and is just annoying overhead for the most simple cases. In addition, even when the modules will be uploaded independently, npm will just flattify the dependency tree (of course you should use versions that are compatible) and everything will be resolved correctly. (as it does now without symlinks) |
|
@ahom yeah I agree this should definitely be fixed. Good point about npm flattening dependencies. For some reason I was under the impression that each module would have all of its dependencies copied in its local node_modules. Your totally right though, the npm install docs actually have a great description of how the flattening works. Would have to make sure that all of the dependent modules are uploaded with the same version numbers but if you control the upload then not really an issue. |
|
@ahom Antoine, I'm running into the same problem. Do you mind sharing your patch until this is officially fixed? |
|
@erykwarren I didn't fork Typescript, I just fixed it locally for this issue to be sure that it was the exact problem. https://github.com/Microsoft/TypeScript/blob/master/src/compiler/sys.ts#L542 return _path.resolve(path);To return _fs.realpathSync(_path.resolve(path));https://github.com/Microsoft/TypeScript/blob/master/src/compiler/program.ts#L44 switch (moduleResolution) {
case ModuleResolutionKind.NodeJs: return nodeModuleNameResolver(moduleName, containingFile, compilerOptions, host);
case ModuleResolutionKind.Classic: return classicNameResolver(moduleName, containingFile, compilerOptions, host);
}To let resolution = null;
switch (moduleResolution) {
case ModuleResolutionKind.NodeJs:
resolution = nodeModuleNameResolver(moduleName, containingFile, compilerOptions, host);
break;
case ModuleResolutionKind.Classic:
resolution = classicNameResolver(moduleName, containingFile, compilerOptions, host);
break;
}
if (resolution
&& resolution.resolvedModule
&& resolution.resolvedModule.isExternalLibraryImport === true
&& resolution.resolvedModule.resolvedFileName) {
resolution.resolvedModule.resolvedFileName = ts.sys.resolvePath(resolution.resolvedModule.resolvedFileName);
}
return resolution; |
|
That worked. Merci beaucoup @ahom |
@ahom @andreialecu @sebastianhoitz Regarding this issue and ts-loader, I believe I've addressed it in the latest version if you want to try giving that another shot. ts-loader sort of sidesteps the issue because it mostly relies on webpack for resolution (which returns the real path), but there was the caching issue mentioned above. I think there is still a bug in TypeScript for this, as shown by @ahom, where the resolved filename coming out of |
|
@jbrantly I can confirm that ts-loader 0.8.0 fixes the issue about the cache! Thanks. 👍 So right now, |
|
should be addressed by #8486 |
|
Hi. I'm using TypeScript 2.0.0-dev.20160703 with vscode 1.2.1 (With typescript.tsdk path set accordingly, so vscode also uses TypeScript 2.0) and I have the problem that I can reproduce this error in vscode. The error is displayed when I save the file but the error disappears when I compile the project with Ctrl-Shift-B. I can successfully use other TypeScript 2.0 features like strictNull-checks so I'm pretty sure I configured vscode correctly. I know, this bug report is for TypeScript, not for vscode, but does anyone know if vscode has its own module resolver so this bug must be fixed in vscode separately? |
|
@kayahr please file a new issue and provide a sample project that we can use to reproduce the issue. |
|
@mhegazy I will do. But I still wonder: Does vscode completely rely on the TypeScript version specified in the typescript.tsdk setting for code validation in the editor (So in theory it should always show the same errors as the command line compiler) or does it use own code (Maybe copied from the TypeScript project) to achieve this? So maybe the bug automatically disappears as soon as vscode is officially using TypeScript 2.0 and there is no need for a bug report. And where should I file the new issue? For the TypeScript project or the vscode project? I'm unsure which project is responsible here. |
it uses the one you specify for everything. so if there is an issue it this issue tracker is the correct place. |
Error
While compiling a project that depends on local packages (symlinked from node_modules folder) I'm getting the following error even though both types are exactly the same.
Symlinks setup
Here I'm trying to compile the debug/sql_to_mongo project
The problem seems to be happening because when resolving the llqm-core module from
debug/sql_to_mongo, frontend/sql and backend/mongothey each point, from Typescript's perspective, to different folders, but they really all resolve to the same one.Compiling with --listFiles to show the dependencies being resolved
Hacky fix
By modifying the resolution of external modules to follow symlinks (with
fs.realpath) it is now compiling properly.Right now I:
fs.realpathcall tots.sys.resolvePath(for node)ts.resolveModuleNameonly when it is an external importI'm not too sure if this is the right way to go.
--listFiles after fix
Thanks!
The text was updated successfully, but these errors were encountered: