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

"error TS2304: Cannot find name" after updating to 7.0.0 #615

Closed
ellipticaldoor opened this Issue Jun 23, 2018 · 41 comments

Comments

Projects
None yet
@ellipticaldoor

ellipticaldoor commented Jun 23, 2018

I updated ts-node from 6.2.0 to 7.0.0 and I started to get the following error:

TSError: ⨯ Unable to compile TypeScript:
src/common/atlas/atlas.constants.ts(3,27): error TS2304: Cannot find name 'Tile'.
src/common/atlas/atlas.constants.ts(6,31): error TS2304: Cannot find name 'ITileLayer'.
src/common/atlas/atlas.constants.ts(27,28): error TS2304: Cannot find name 'ITileLayer'.

    at createTSError (/Users/miguel/src/later/node_modules/ts-node/src/index.ts:261:12)
    at getOutput (/Users/miguel/src/later/node_modules/ts-node/src/index.ts:367:40)
    at Object.compile (/Users/miguel/src/later/node_modules/ts-node/src/index.ts:557:11)
    at Module.m._compile (/Users/miguel/src/later/node_modules/ts-node/src/index.ts:439:43)
    at Module._extensions..js (internal/modules/cjs/loader.js:713:10)
    at require.extensions.(anonymous function) (/Users/miguel/src/later/node_modules/ts-node/src/index.ts:442:12)
    at Object.nodeDevHook [as .ts] (/Users/miguel/src/later/node_modules/node-dev/lib/hook.js:61:7)
    at Module.load (internal/modules/cjs/loader.js:612:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:551:12)
    at Function.Module._load (internal/modules/cjs/loader.js:543:3)
[ERROR] 11:14:48 TSError

It looks like this version is not able to find my custom type definitions. I recently started to learn typescript so I'm not sure if what I'm doing is good practice, this is my folder structure

image

Y put my types inside *.d.ts files and nothing more, this worked before updating.

Not sure if this can help but here is my .tsconfig

{
	"compilerOptions": {
		"strict": true,
		"alwaysStrict": true,
		"module": "commonjs",
		"moduleResolution": "node",
		"types": ["node", "jest", "pixi.js", "matter-js"],
		"newLine": "LF",
		"outDir": "lib",
		"jsx": "preserve",
		"target": "es2017",
		"lib": [
			"es2017",
			"dom"
		],
		"noUnusedLocals": true,
		"noUnusedParameters": true,
		"noImplicitAny": true,
		"noFallthroughCasesInSwitch": true,
		"experimentalDecorators": true,
		"baseUrl": "./src",
		"paths": {
			"common/*": [
				"./common/*"
			],
			"client/*": [
				"./client/*"
			],
			"server/*": [
				"./server/*"
			]
		}
	},
	"include": [
		"src/common/**/*.ts",
		"src/client/**/*.ts"
	],
	"exclude": [
		".git",
		"node_modules"
	]
}

@ellipticaldoor ellipticaldoor changed the title from error TS2304: Cannot find name after updating to 7.0.0 to "error TS2304: Cannot find name" after updating to 7.0.0 Jun 23, 2018

@polco

This comment has been minimized.

Show comment
Hide comment
@polco

polco Jun 23, 2018

I have the same issue, my custom types files are not loaded anymore with the version 7.

polco commented Jun 23, 2018

I have the same issue, my custom types files are not loaded anymore with the version 7.

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jun 23, 2018

Member

A major version means breaking changes. If something breaks, you can check the changelog or commits to see what happened. See https://github.com/TypeStrong/ts-node/releases/tag/v7.0.0 and the notice in the README. If they aren't clear, feel free to clarify them. If you'd like the old behaviour for now, you can use ts-node --files.

Edit: Reference to issue which prompted changing the behaviour - gulpjs/interpret#51.

Member

blakeembrey commented Jun 23, 2018

A major version means breaking changes. If something breaks, you can check the changelog or commits to see what happened. See https://github.com/TypeStrong/ts-node/releases/tag/v7.0.0 and the notice in the README. If they aren't clear, feel free to clarify them. If you'd like the old behaviour for now, you can use ts-node --files.

Edit: Reference to issue which prompted changing the behaviour - gulpjs/interpret#51.

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jun 23, 2018

Member

@weswigham Would you be kind enough to share the recommended way someone should include these files into their ts-node project (without --files)?

Member

blakeembrey commented Jun 23, 2018

@weswigham Would you be kind enough to share the recommended way someone should include these files into their ts-node project (without --files)?

@ellipticaldoor

This comment has been minimized.

Show comment
Hide comment
@ellipticaldoor

ellipticaldoor Jun 24, 2018

Yes, I would like to know how is the proper way.

ellipticaldoor commented Jun 24, 2018

Yes, I would like to know how is the proper way.

@jeremejevs

This comment has been minimized.

Show comment
Hide comment
@jeremejevs

jeremejevs Jun 25, 2018

Why make a major release so quickly? I do agree that, as @weswigham says, "the build orchestrator is not part of the build it orchestrates", but the fix is kind of on the extreme side. Why not at least try taking some useful parts out of the tsconfig.json, if there is one? esModuleInterop, lib, typeRoots, exclude, the strictness settings, etc.

Sticking to v6 for now, since I don't want to:

  • Maintain a separate tsconfig.json just for ts-node.
  • Augment every single ts-node invocation with a --project.
  • Use triple-slash pragmas where I previously didn't have to.

It's possible that I'm in the tiny minority here, but the discussion was way too short for this change, IMO.

jeremejevs commented Jun 25, 2018

Why make a major release so quickly? I do agree that, as @weswigham says, "the build orchestrator is not part of the build it orchestrates", but the fix is kind of on the extreme side. Why not at least try taking some useful parts out of the tsconfig.json, if there is one? esModuleInterop, lib, typeRoots, exclude, the strictness settings, etc.

Sticking to v6 for now, since I don't want to:

  • Maintain a separate tsconfig.json just for ts-node.
  • Augment every single ts-node invocation with a --project.
  • Use triple-slash pragmas where I previously didn't have to.

It's possible that I'm in the tiny minority here, but the discussion was way too short for this change, IMO.

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jun 25, 2018

Member

@jeremejevs You assume I haven't thought about this for the past three years of working on this module, which I (unfortunately) have. There's been many iterations on the "correct" approach, but I agreed this one was the most appropriate today given types and other TypeScript features introduced since 2.0.

Why not at least try taking some useful parts out of the tsconfig.json, if there is one? esModuleInterop, lib, typeRoots, exclude, the strictness settings, etc.

You can't just magically introduce a flag that controls which files get type checked. I'm not sure how you believe those flags would play into this? Are you expecting ts-node to mess with your settings (which, FWIW, is exactly what this is doing and you're objecting to). Unfortunately there's never been a one-size fits all for the single entry point to ts-node since there's so many varied use-cases, this is just aiming for the most universal use-case with convenient defaults.

You don't even need to maintain a separate tsconfig.json for ts-node - this changed works for all my projects using ts-node without separate configuration. What's not working for you?

Member

blakeembrey commented Jun 25, 2018

@jeremejevs You assume I haven't thought about this for the past three years of working on this module, which I (unfortunately) have. There's been many iterations on the "correct" approach, but I agreed this one was the most appropriate today given types and other TypeScript features introduced since 2.0.

Why not at least try taking some useful parts out of the tsconfig.json, if there is one? esModuleInterop, lib, typeRoots, exclude, the strictness settings, etc.

You can't just magically introduce a flag that controls which files get type checked. I'm not sure how you believe those flags would play into this? Are you expecting ts-node to mess with your settings (which, FWIW, is exactly what this is doing and you're objecting to). Unfortunately there's never been a one-size fits all for the single entry point to ts-node since there's so many varied use-cases, this is just aiming for the most universal use-case with convenient defaults.

You don't even need to maintain a separate tsconfig.json for ts-node - this changed works for all my projects using ts-node without separate configuration. What's not working for you?

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jun 25, 2018

Member

FWIW, I'm going to have to assume you're having issues with trying to "global" types into your project. Those aren't particularly recommended today as it makes your code fragile to conflicting types and unable to interop with dependents. You can instead put your types in modules, export and import them where needed instead of having them "everywhere".

Another thing, you can use ts-node --files to include "all" files if that's what you really want. You just have to be aware project startup can be slow. The new default alleviates some issues such as, in the past, you could have ts-node stall forever by spawning the REPL in the home directory (causing TypeScript to, by default, crawl everything in the directory looking for types).

Edit: The existing issue for reference (note it never even finished):

image

Member

blakeembrey commented Jun 25, 2018

FWIW, I'm going to have to assume you're having issues with trying to "global" types into your project. Those aren't particularly recommended today as it makes your code fragile to conflicting types and unable to interop with dependents. You can instead put your types in modules, export and import them where needed instead of having them "everywhere".

Another thing, you can use ts-node --files to include "all" files if that's what you really want. You just have to be aware project startup can be slow. The new default alleviates some issues such as, in the past, you could have ts-node stall forever by spawning the REPL in the home directory (causing TypeScript to, by default, crawl everything in the directory looking for types).

Edit: The existing issue for reference (note it never even finished):

image

@jeremejevs

This comment has been minimized.

Show comment
Hide comment
@jeremejevs

jeremejevs Jun 25, 2018

@blakeembrey Poor reading skills this time of day + didn't look at the diff. Interpreted "Skip loading tsconfig.json files by default to speed up startup" as "tsconfig.json is ignored now". Sorry.

What do you mean by "global" types? I have a few of these:

// typings/some-npm-package.d.ts
declare module "some-npm-package" {
  // ...
}

And typeRoots: [./typings] in the config. v6 sees them, v7 doesn't.

jeremejevs commented Jun 25, 2018

@blakeembrey Poor reading skills this time of day + didn't look at the diff. Interpreted "Skip loading tsconfig.json files by default to speed up startup" as "tsconfig.json is ignored now". Sorry.

What do you mean by "global" types? I have a few of these:

// typings/some-npm-package.d.ts
declare module "some-npm-package" {
  // ...
}

And typeRoots: [./typings] in the config. v6 sees them, v7 doesn't.

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jun 25, 2018

Member

Feel free to submit corrections to how it's phrased. I know I can be terrible at that and don't typically spend enough time thinking about all the ways it's interpreted.

As for typeRoots, it should still be working - let me create double check for you, give me 15 minutes or so.

Update: Failing to get my test case to fail, so haven't been able to replicate the issue yet.
Update 2: Stupid me. Left the cache enabled so after it worked once it worked forever.

Member

blakeembrey commented Jun 25, 2018

Feel free to submit corrections to how it's phrased. I know I can be terrible at that and don't typically spend enough time thinking about all the ways it's interpreted.

As for typeRoots, it should still be working - let me create double check for you, give me 15 minutes or so.

Update: Failing to get my test case to fail, so haven't been able to replicate the issue yet.
Update 2: Stupid me. Left the cache enabled so after it worked once it worked forever.

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jun 25, 2018

Member

@jeremejevs Found the issue - looks like your typeRoots wasn't doing anything because it needs to be a directory and index.d.ts (e.g. typings/some-npm-package/index.d.ts). Looks like it worked because you have include or something else globbing it to be included. This has been a fun exercise, spotted a bug with the cache flag after much head banging, and learned more about types and typeRoots.

Member

blakeembrey commented Jun 25, 2018

@jeremejevs Found the issue - looks like your typeRoots wasn't doing anything because it needs to be a directory and index.d.ts (e.g. typings/some-npm-package/index.d.ts). Looks like it worked because you have include or something else globbing it to be included. This has been a fun exercise, spotted a bug with the cache flag after much head banging, and learned more about types and typeRoots.

@salemdar

This comment has been minimized.

Show comment
Hide comment
@salemdar

salemdar Jun 25, 2018

How can we pass files flag when we are using TypeScript for gulp tasks?

salemdar commented Jun 25, 2018

How can we pass files flag when we are using TypeScript for gulp tasks?

@lddubeau

This comment has been minimized.

Show comment
Hide comment
@lddubeau

lddubeau Jun 25, 2018

@salemdar Maybe I missed something but I do not see any way to get gulp to just pick up an additional files flag to pass to ts-node. The workaround I use is:

  1. Move gulpfile.ts to a different name. Note that this is still a TypeScript file. It is just no longer named gulpfile.ts.

  2. Write a gulpfile.js that initializes ts-node as I wish and then loads my renamed gulpfile.ts.

This is what I'm using for the gulpfile.js:

"use strict";

require("ts-node").register({
  transpileOnly: true,
});

require("./gulptasks/main"); // Load the renamed gulpfile.ts

I've decided to go with transpileOnly: true to fix my problem. I figured that there was no benefit to having ts-node check types during a Gulp run. I already have an IDE that does type checking on the fly, which catches most problems (and I expect any collaborator to have something equivalent to my setup), and I have a tslint task that includes type-checking, which would catch things I may not notice through the IDE.

If you do want Gulp to run type checking and use files then you can just replace transpileOnly: true, with files: true. I've tested both and they both worked around the compilation errors I got after upgrading to ts-node 7.0.0.

lddubeau commented Jun 25, 2018

@salemdar Maybe I missed something but I do not see any way to get gulp to just pick up an additional files flag to pass to ts-node. The workaround I use is:

  1. Move gulpfile.ts to a different name. Note that this is still a TypeScript file. It is just no longer named gulpfile.ts.

  2. Write a gulpfile.js that initializes ts-node as I wish and then loads my renamed gulpfile.ts.

This is what I'm using for the gulpfile.js:

"use strict";

require("ts-node").register({
  transpileOnly: true,
});

require("./gulptasks/main"); // Load the renamed gulpfile.ts

I've decided to go with transpileOnly: true to fix my problem. I figured that there was no benefit to having ts-node check types during a Gulp run. I already have an IDE that does type checking on the fly, which catches most problems (and I expect any collaborator to have something equivalent to my setup), and I have a tslint task that includes type-checking, which would catch things I may not notice through the IDE.

If you do want Gulp to run type checking and use files then you can just replace transpileOnly: true, with files: true. I've tested both and they both worked around the compilation errors I got after upgrading to ts-node 7.0.0.

@jeremejevs

This comment has been minimized.

Show comment
Hide comment
@jeremejevs

jeremejevs Jun 25, 2018

@blakeembrey Thank you for taking time to look into this. Decided to investigate this myself, and wow, still got some misconceptions. Everything was working, so I was sure I had it all figured out...

Indeed, typeRoots was doing nothing. The typings were being picked up by TypeScript's default include (my tsconfig.json has only an exclude). It would work if the typings did have the right shape, but typeRoots is for global declarations, paths is a better fit for that. Which is conveniently omitted in tsconfig.json docs, and is instead described in module resolutions docs.

The "fix" is to put this in place of typeRoots:

"baseUrl": ".",
"paths": { "*" : ["typings/*"] }

And translate all typings from typings/*.d.ts to typings/*/index.d.ts, i.e. from this:

// typings/some-npm-package.d.ts
declare module "some-npm-package" {
  const x: string;
  export = x;
}

To this:

// typings/some-npm-package/index.d.ts
declare const x: string;
export = x;

I'm choosing this approach because I always wanted my typings to look the same as what you would find on DefinitelyTyped, and now it's possible, and it works with both ts-node and tsc.

I also have typings like this:

// typings/svg.d.ts
declare module "*.svg" {
  const x: string;
  export default x;
}

These are okay to leave as-is, since they make sense only in the context of a bundler like Webpack, and ts-node shouldn't be able to execute files which import SVGs and such. One exception is JSON, but there's a setting for it now in TS 2.9. This way, there's no need in involving typeRoots at all.

I'm glad this change broke my stuff, now I have a better config and a better understanding of it, thanks.

Edit

Have fiddled around some more, and found out that paths works with declare module 'some-npm-package' too, so if you're in a rush, you don't even have to do the translation part, just make sure the directory matches the module.

Adding typings/*/index.d.ts to exclude can help with some typos/mistakes.

jeremejevs commented Jun 25, 2018

@blakeembrey Thank you for taking time to look into this. Decided to investigate this myself, and wow, still got some misconceptions. Everything was working, so I was sure I had it all figured out...

Indeed, typeRoots was doing nothing. The typings were being picked up by TypeScript's default include (my tsconfig.json has only an exclude). It would work if the typings did have the right shape, but typeRoots is for global declarations, paths is a better fit for that. Which is conveniently omitted in tsconfig.json docs, and is instead described in module resolutions docs.

The "fix" is to put this in place of typeRoots:

"baseUrl": ".",
"paths": { "*" : ["typings/*"] }

And translate all typings from typings/*.d.ts to typings/*/index.d.ts, i.e. from this:

// typings/some-npm-package.d.ts
declare module "some-npm-package" {
  const x: string;
  export = x;
}

To this:

// typings/some-npm-package/index.d.ts
declare const x: string;
export = x;

I'm choosing this approach because I always wanted my typings to look the same as what you would find on DefinitelyTyped, and now it's possible, and it works with both ts-node and tsc.

I also have typings like this:

// typings/svg.d.ts
declare module "*.svg" {
  const x: string;
  export default x;
}

These are okay to leave as-is, since they make sense only in the context of a bundler like Webpack, and ts-node shouldn't be able to execute files which import SVGs and such. One exception is JSON, but there's a setting for it now in TS 2.9. This way, there's no need in involving typeRoots at all.

I'm glad this change broke my stuff, now I have a better config and a better understanding of it, thanks.

Edit

Have fiddled around some more, and found out that paths works with declare module 'some-npm-package' too, so if you're in a rush, you don't even have to do the translation part, just make sure the directory matches the module.

Adding typings/*/index.d.ts to exclude can help with some typos/mistakes.

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jun 25, 2018

Member

I’m not as glad it broke things for people (it’s one of many “the lesser of two evils” but trying to figure out which evil is which is the hard part), but I’m thankful for your investigation work!

Member

blakeembrey commented Jun 25, 2018

I’m not as glad it broke things for people (it’s one of many “the lesser of two evils” but trying to figure out which evil is which is the hard part), but I’m thankful for your investigation work!

@g-harel

This comment has been minimized.

Show comment
Hide comment
@g-harel

g-harel Jun 26, 2018

I really feel like there should be more context in the release notes for a major version.

g-harel commented Jun 26, 2018

I really feel like there should be more context in the release notes for a major version.

@dsifford

This comment has been minimized.

Show comment
Hide comment
@dsifford

dsifford Jun 26, 2018

For the lazy webpack users that are running into this issue:

Assuming the following....

  1. You don't feel like refactoring.
  2. You are starting webpack using an npm script like this...
{
  "scripts": {
    "start": "webpack"
  }
}

Simply change it to this and it'll work like before:

{
  "scripts": {
    "start": "TS_NODE_FILES=true webpack"
  }
}

dsifford commented Jun 26, 2018

For the lazy webpack users that are running into this issue:

Assuming the following....

  1. You don't feel like refactoring.
  2. You are starting webpack using an npm script like this...
{
  "scripts": {
    "start": "webpack"
  }
}

Simply change it to this and it'll work like before:

{
  "scripts": {
    "start": "TS_NODE_FILES=true webpack"
  }
}
@freezy

This comment has been minimized.

Show comment
Hide comment
@freezy

freezy Jun 27, 2018

@lddubeau: You can also set an environment variable instead of --files, which will then be picked up through Gulp.

@blakeembrey: Whichever evil you choose, describing it more explicitly in the release notes would have saved us all a lot of time!

freezy commented Jun 27, 2018

@lddubeau: You can also set an environment variable instead of --files, which will then be picked up through Gulp.

@blakeembrey: Whichever evil you choose, describing it more explicitly in the release notes would have saved us all a lot of time!

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jun 27, 2018

Member

Feel free to submit an issue with a better description that you’d like to see.

Member

blakeembrey commented Jun 27, 2018

Feel free to submit an issue with a better description that you’d like to see.

@freezy

This comment has been minimized.

Show comment
Hide comment
@freezy

freezy Jun 27, 2018

From my admittedly limited understanding, maybe:

If your project contains merge declarations that aren't in the dependency tree, it's possible to fall back to the previous behavior by setting the --files flag or the TS_NODE_FILES environment variable.

But then, putting the declarations in the correct folder like @jeremejevs suggests seems to work too.

EDIT: I haven't been using the --files flag before, but for people who did, a note about --files doing essentially the contrary now would surely be helpful as well.

freezy commented Jun 27, 2018

From my admittedly limited understanding, maybe:

If your project contains merge declarations that aren't in the dependency tree, it's possible to fall back to the previous behavior by setting the --files flag or the TS_NODE_FILES environment variable.

But then, putting the declarations in the correct folder like @jeremejevs suggests seems to work too.

EDIT: I haven't been using the --files flag before, but for people who did, a note about --files doing essentially the contrary now would surely be helpful as well.

@ljani

This comment has been minimized.

Show comment
Hide comment
@ljani

ljani Jun 29, 2018

Contributor

@jeremejevs You are not using tsconfig-paths with that configuration, are you? Because it looks like tsconfig-paths seems to think that typings/some-npm-package should also contain index.js file:

Error: Cannot find module 'C:\...\my-project\types\postcss-nested'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:594:15)
    at Function.Module._resolveFilename (C:\...\my-project\node_modules\tsconfig-paths\lib\register.js:29:44)
    at Function.Module._load (internal/modules/cjs/loader.js:520:25)
    at Module.require (internal/modules/cjs/loader.js:650:17)
    at require (internal/modules/cjs/helpers.js:20:18)
...

I'm facing this problem when my webpackfile.ts needs ambient typings for postcss-nested:

webpack-dev-server --config-register tsconfig-paths/register
Contributor

ljani commented Jun 29, 2018

@jeremejevs You are not using tsconfig-paths with that configuration, are you? Because it looks like tsconfig-paths seems to think that typings/some-npm-package should also contain index.js file:

Error: Cannot find module 'C:\...\my-project\types\postcss-nested'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:594:15)
    at Function.Module._resolveFilename (C:\...\my-project\node_modules\tsconfig-paths\lib\register.js:29:44)
    at Function.Module._load (internal/modules/cjs/loader.js:520:25)
    at Module.require (internal/modules/cjs/loader.js:650:17)
    at require (internal/modules/cjs/helpers.js:20:18)
...

I'm facing this problem when my webpackfile.ts needs ambient typings for postcss-nested:

webpack-dev-server --config-register tsconfig-paths/register
@jeremejevs

This comment has been minimized.

Show comment
Hide comment
@jeremejevs

jeremejevs Jun 29, 2018

@ljani Sorry, I'm not. It appears that my "fix" clashes with the assumptions that tsconfig-paths is making about the usage of paths. The only workaround that comes to my mind is to have a separate tsconfig.json for now, which doesn't have these baseUrl/path changes. But I think this should be solved on their side, maybe by adding a flag, which lets you tell it to ignore certain paths keys or something like that. Either way, opening an issue there shouldn't hurt.

jeremejevs commented Jun 29, 2018

@ljani Sorry, I'm not. It appears that my "fix" clashes with the assumptions that tsconfig-paths is making about the usage of paths. The only workaround that comes to my mind is to have a separate tsconfig.json for now, which doesn't have these baseUrl/path changes. But I think this should be solved on their side, maybe by adding a flag, which lets you tell it to ignore certain paths keys or something like that. Either way, opening an issue there shouldn't hurt.

@ljani

This comment has been minimized.

Show comment
Hide comment
@ljani

ljani Jun 29, 2018

Contributor

@jeremejevs Thanks for the confirmation! I wanted to make sure it was not a misconfiguration on my part.

Contributor

ljani commented Jun 29, 2018

@jeremejevs Thanks for the confirmation! I wanted to make sure it was not a misconfiguration on my part.

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jun 29, 2018

Member

@ljani How did you use these definitions before? If they're still global (e.g. declare module "x"), you can use typeRoots. If they're modules, I'm not sure how you could have made TypeScript discover them before because the module file doesn't directly map to a module name without paths configured.

Member

blakeembrey commented Jun 29, 2018

@ljani How did you use these definitions before? If they're still global (e.g. declare module "x"), you can use typeRoots. If they're modules, I'm not sure how you could have made TypeScript discover them before because the module file doesn't directly map to a module name without paths configured.

@ljani

This comment has been minimized.

Show comment
Hide comment
@ljani

ljani Jun 29, 2018

Contributor

They're global. I didn't even try typeRoots, because of a previous bad experience from misunderstanding them and @jeremejevs was so convincing, that I decided to try his solution first. If I specify typeRoots: ["./typings", "./node_modules/@types"], will it include types from eg. ./node_modules/@material-ui/core/index.d.ts, if I import a module from @material-ui/core? (I cannot test it at the moment.)

Contributor

ljani commented Jun 29, 2018

They're global. I didn't even try typeRoots, because of a previous bad experience from misunderstanding them and @jeremejevs was so convincing, that I decided to try his solution first. If I specify typeRoots: ["./typings", "./node_modules/@types"], will it include types from eg. ./node_modules/@material-ui/core/index.d.ts, if I import a module from @material-ui/core? (I cannot test it at the moment.)

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jun 29, 2018

Member

@ljani That should work 👍

Member

blakeembrey commented Jun 29, 2018

@ljani That should work 👍

@ljani

This comment has been minimized.

Show comment
Hide comment
@ljani

ljani Jun 29, 2018

Contributor

@blakeembrey Great! Thanks for letting me know. The first time I tried TypeScript, I think I forgot to include ./node_modules/@types in typeRoots and thus it seemed to break everything as you might expect. It has been a while :-)

Contributor

ljani commented Jun 29, 2018

@blakeembrey Great! Thanks for letting me know. The first time I tried TypeScript, I think I forgot to include ./node_modules/@types in typeRoots and thus it seemed to break everything as you might expect. It has been a while :-)

@ljani

This comment has been minimized.

Show comment
Hide comment
@ljani

ljani Jun 29, 2018

Contributor

@blakeembrey It works, thanks again. Do you happen to know why at least TS language service gets confused if typeRoots contains wildcard modules? I'm trying to share eg. this one between two modules:

declare module "*.png";

It works fine if I add the file directly to the modules.

EDIT: Nvm, it might be something else. It seems that my tsconfig.json meant for the node side tries to parse types in node_modules/@types, which are only meant for web. I'll have to take a closer look.
EDIT2: Switched to explicitly specifying types and it's all good it seems.
EDIT3: In the end, I ended up adding both, typeRoots and the paths suggestion by @jeremejevs (see also this issue) after dividab/tsconfig-paths#51 was fixed (thanks Jonas!). Thanks all, this made me as well to understand much better how TypeScript resolves type definitions! It's important to make a difference how global declarations (*.png) are handled by typeRoots and module declarations (postcss-nested) by paths.

Contributor

ljani commented Jun 29, 2018

@blakeembrey It works, thanks again. Do you happen to know why at least TS language service gets confused if typeRoots contains wildcard modules? I'm trying to share eg. this one between two modules:

declare module "*.png";

It works fine if I add the file directly to the modules.

EDIT: Nvm, it might be something else. It seems that my tsconfig.json meant for the node side tries to parse types in node_modules/@types, which are only meant for web. I'll have to take a closer look.
EDIT2: Switched to explicitly specifying types and it's all good it seems.
EDIT3: In the end, I ended up adding both, typeRoots and the paths suggestion by @jeremejevs (see also this issue) after dividab/tsconfig-paths#51 was fixed (thanks Jonas!). Thanks all, this made me as well to understand much better how TypeScript resolves type definitions! It's important to make a difference how global declarations (*.png) are handled by typeRoots and module declarations (postcss-nested) by paths.

@blakeembrey blakeembrey added the invalid label Jul 2, 2018

@19majkel94

This comment has been minimized.

Show comment
Hide comment
@19majkel94

19majkel94 Jul 12, 2018

What about ambient module declaration for module augmentation?

declare namespace NodeJS {
  interface Global {
    TypeGraphQLMetadataStorage: import("../metadata/metadata-storage").MetadataStorage;
  }
}

I've tried the typeRoots and paths tricks but no success 😞

19majkel94 commented Jul 12, 2018

What about ambient module declaration for module augmentation?

declare namespace NodeJS {
  interface Global {
    TypeGraphQLMetadataStorage: import("../metadata/metadata-storage").MetadataStorage;
  }
}

I've tried the typeRoots and paths tricks but no success 😞

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jul 13, 2018

Member

@19majkel94 Can you share your configuration so we can see why it didn't work?

@ljani Thanks for your investigations! It'd be worth adding a section to the README on how to set this up correctly, I'll try that tonight 😄

Member

blakeembrey commented Jul 13, 2018

@19majkel94 Can you share your configuration so we can see why it didn't work?

@ljani Thanks for your investigations! It'd be worth adding a section to the README on how to set this up correctly, I'll try that tonight 😄

@19majkel94

This comment has been minimized.

Show comment
Hide comment
@19majkel94

19majkel94 Jul 14, 2018

@blakeembrey It doesn't work because typeRoots and paths are used only for module resolution for looking when using import, not when the module is found but additional declarations are in other file in the project.

Here's quick reproduction gist:
https://gist.github.com/19majkel94/4799a197d238639752a29333a78cc7d9

19majkel94 commented Jul 14, 2018

@blakeembrey It doesn't work because typeRoots and paths are used only for module resolution for looking when using import, not when the module is found but additional declarations are in other file in the project.

Here's quick reproduction gist:
https://gist.github.com/19majkel94/4799a197d238639752a29333a78cc7d9

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jul 14, 2018

Member

@19majkel94 That's not true, typeRoots is for global definitions.

Member

blakeembrey commented Jul 14, 2018

@19majkel94 That's not true, typeRoots is for global definitions.

@19majkel94

This comment has been minimized.

Show comment
Hide comment
@19majkel94

19majkel94 Jul 14, 2018

@blakeembrey So any idea why the gist is not working? 😕

19majkel94 commented Jul 14, 2018

@blakeembrey So any idea why the gist is not working? 😕

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jul 14, 2018

Member

@19majkel94 There's documentation and links in this thread pointing out what you've run into. You need to make it a directory and use index.d.ts.

Member

blakeembrey commented Jul 14, 2018

@19majkel94 There's documentation and links in this thread pointing out what you've run into. You need to make it a directory and use index.d.ts.

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Jul 14, 2018

Member

I'll be working this week on improving documentation/introduction on how to use these features in TypeScript - it's definitely not clear from the TypeScript side given the number of comments/issues.

Member

blakeembrey commented Jul 14, 2018

I'll be working this week on improving documentation/introduction on how to use these features in TypeScript - it's definitely not clear from the TypeScript side given the number of comments/issues.

@19majkel94

This comment has been minimized.

Show comment
Hide comment
@19majkel94

19majkel94 Jul 14, 2018

Thanks, it's awful but at least it works - I wonder how deno solves that problem 🤔

19majkel94 commented Jul 14, 2018

Thanks, it's awful but at least it works - I wonder how deno solves that problem 🤔

@samjmckenzie

This comment has been minimized.

Show comment
Hide comment
@samjmckenzie

samjmckenzie Jul 27, 2018

@ljani Could I possibly see your tsconfig.json configuration? I can't get mine to resolve types that aren't in node_modules/@types.

samjmckenzie commented Jul 27, 2018

@ljani Could I possibly see your tsconfig.json configuration? I can't get mine to resolve types that aren't in node_modules/@types.

@ljani

This comment has been minimized.

Show comment
Hide comment
@ljani

ljani Jul 28, 2018

Contributor

@samjmckenzie Sure, this one is from a monorepo where the package.json is above all of the packages. The tsconfig.json file is located in src and there's a types directory next to the src folder:

{
  "extends": "../../../tools/tsconfig-web.json",
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "*": [
        "*",
        "../types/*"
      ]
    },
    "typeRoots": [
      "../../../node_modules/@types",
      "../types"
    ]
  }
}

The types folder contains other folders with the module names and those contain index.d.ts files.

In tsconfig-web.json, "moduleResolution": "node" is the only thing, which affects path resolution. But in addition I'm using and "lib": ["esnext", "dom"], allowSyntheticDefaultImports and esModuleInterop there.

Hope this helps.

Contributor

ljani commented Jul 28, 2018

@samjmckenzie Sure, this one is from a monorepo where the package.json is above all of the packages. The tsconfig.json file is located in src and there's a types directory next to the src folder:

{
  "extends": "../../../tools/tsconfig-web.json",
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "*": [
        "*",
        "../types/*"
      ]
    },
    "typeRoots": [
      "../../../node_modules/@types",
      "../types"
    ]
  }
}

The types folder contains other folders with the module names and those contain index.d.ts files.

In tsconfig-web.json, "moduleResolution": "node" is the only thing, which affects path resolution. But in addition I'm using and "lib": ["esnext", "dom"], allowSyntheticDefaultImports and esModuleInterop there.

Hope this helps.

@samjmckenzie

This comment has been minimized.

Show comment
Hide comment
@samjmckenzie

samjmckenzie Jul 28, 2018

Thanks, but unfortunately I can't get it working. Oh well. I will just have to keep using the --files option.

samjmckenzie commented Jul 28, 2018

Thanks, but unfortunately I can't get it working. Oh well. I will just have to keep using the --files option.

ludviglundgren added a commit to ludviglundgren/graphql-typescript-server that referenced this issue Aug 15, 2018

bkazi added a commit to bkazi/tfjs-core that referenced this issue Aug 19, 2018

Add files option to ts-node
Fixes failing tests on node 10
TypeStrong/ts-node#615
@ljani

This comment has been minimized.

Show comment
Hide comment
@ljani

ljani Aug 27, 2018

Contributor

Actually the way paths is used in my example has at least one drawback. For example webpack seems to be making require("crypto") calls and this yields thousands of disk lookups for crypto, crypto.js, crypto.ts etc in the two paths. I'm not sure what's the performance impact, but I thought you guys should know.

Also now that I come to think about this once agan, is this test wrong? Shouldn't it be expect(err).not.to.match(/Error: Cannot find module 'does-not-exist'/), because the typings do exist?

EDIT: After initial testing, my build time was reduced from ~120 seconds to ~75 seconds after I removed the * path and moved the types to node_modules/@types, so the performance impact seems to be huge.

Contributor

ljani commented Aug 27, 2018

Actually the way paths is used in my example has at least one drawback. For example webpack seems to be making require("crypto") calls and this yields thousands of disk lookups for crypto, crypto.js, crypto.ts etc in the two paths. I'm not sure what's the performance impact, but I thought you guys should know.

Also now that I come to think about this once agan, is this test wrong? Shouldn't it be expect(err).not.to.match(/Error: Cannot find module 'does-not-exist'/), because the typings do exist?

EDIT: After initial testing, my build time was reduced from ~120 seconds to ~75 seconds after I removed the * path and moved the types to node_modules/@types, so the performance impact seems to be huge.

@blakeembrey

This comment has been minimized.

Show comment
Hide comment
@blakeembrey

blakeembrey Aug 27, 2018

Member

@ljani The test is correct. That’s a runtime error, not a compilation error. As for lookups, just don’t use the wildcard too heavily - there’s a difference between paths and types too.

Member

blakeembrey commented Aug 27, 2018

@ljani The test is correct. That’s a runtime error, not a compilation error. As for lookups, just don’t use the wildcard too heavily - there’s a difference between paths and types too.

@ljani

This comment has been minimized.

Show comment
Hide comment
@ljani

ljani Aug 28, 2018

Contributor

Ah, right. However, if I remove the whole typings folder (at least on Windows), the test still passes. Any idea why?

EDIT: the output is the same:

ts-node ❯❯❯ get-item .\tests\typings
get-item : Cannot find path 'C:\devel\ts-node\tests\typings' because it does not exist.
ts-node ❯❯❯ node .\dist\bin.js --project .\tests\tsconfig.json .\tests\custom-types.ts
Error: Cannot find module 'does-not-exist'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:581:15)
    at Function.Module._load (internal/modules/cjs/loader.js:507:25)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (C:\devel\ts-node\tests\custom-types.ts:1:1)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Module.m._compile (C:\devel\ts-node\src\index.ts:439:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Object.require.extensions.(anonymous function) [as .ts] (C:\devel\ts-node\src\index.ts:442:12)
    at Module.load (internal/modules/cjs/loader.js:599:32)

EDIT2: If I remove ./typings from tests/tsconfig.json, I do get a different error (tests/custom-types.ts(1,24): error TS2307: Cannot find module 'does-not-exist'.). Is there some kind of cache? Yeah, --no-cache helps this.

EDIT3: After all, I moved back to declare module types.

EDIT4: I don't know. paths seems to be the correct solution, but having a * path causes unacceptable performance degration. I guess I'll just specify the module names myself. I wonder is there a way to tell paths to prioritize Node's built-in modules over filesystem.

Contributor

ljani commented Aug 28, 2018

Ah, right. However, if I remove the whole typings folder (at least on Windows), the test still passes. Any idea why?

EDIT: the output is the same:

ts-node ❯❯❯ get-item .\tests\typings
get-item : Cannot find path 'C:\devel\ts-node\tests\typings' because it does not exist.
ts-node ❯❯❯ node .\dist\bin.js --project .\tests\tsconfig.json .\tests\custom-types.ts
Error: Cannot find module 'does-not-exist'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:581:15)
    at Function.Module._load (internal/modules/cjs/loader.js:507:25)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (C:\devel\ts-node\tests\custom-types.ts:1:1)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Module.m._compile (C:\devel\ts-node\src\index.ts:439:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Object.require.extensions.(anonymous function) [as .ts] (C:\devel\ts-node\src\index.ts:442:12)
    at Module.load (internal/modules/cjs/loader.js:599:32)

EDIT2: If I remove ./typings from tests/tsconfig.json, I do get a different error (tests/custom-types.ts(1,24): error TS2307: Cannot find module 'does-not-exist'.). Is there some kind of cache? Yeah, --no-cache helps this.

EDIT3: After all, I moved back to declare module types.

EDIT4: I don't know. paths seems to be the correct solution, but having a * path causes unacceptable performance degration. I guess I'll just specify the module names myself. I wonder is there a way to tell paths to prioritize Node's built-in modules over filesystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment