nx icon indicating copy to clipboard operation
nx copied to clipboard

Issue in generated tsconfig files for apps & libs

Open johannesschobel opened this issue 4 years ago • 24 comments

Current Behavior

Yesterday, i noticed a strange behavior when working in my newly (!) created nrwl workspace. I develop with the latest VSCode (1.52.x) and was not able to "update imports on file move" anymore (i.e., update the import statements when moving a file).

This issue #3106 is somewhat related to my problem

Expected Behavior

It should be able to update the import statements. This was able in prior versions of nrwl, but i don't know how many versions we need to go back.

Steps to Reproduce

I did a lot of investigations, but here is what i already found out to (at least for me) successfully reproduce the issue:

  1. npx create-nx-workspace@latest foo (this should install version 11.1.x)
  2. select an empty project
  3. add a file a.ts and b.ts to the apps folder. Import b into a and then move b to another folder. this should work and the import statements should get properly updated :green_circle:
  4. now add a library with npx nx generate @nrwl/workspace:lib bar
  5. again, create a file a.ts and b.ts within this newly created lib and import b within a. Then move b, however, the import is not updated :red_circle:
  6. this now also fails for the files created in step 3 (within the apps directory!)
  7. now go to the libs/bar/tsconfig.json file and remove the includes and files attribute.
  8. now go to the libs/bar/tsconfig.lib.json and libs/bar/tsconfig.spec.json and add a composite: true and declaration: true attribute to the compilerOptions.
  9. save everything and try to move files again - it should now work again as expected :green_circle:

Environment

I use the latest version of nrwl/nx

johannesschobel avatar Jan 12 '21 09:01 johannesschobel

ok, i digged a bit deeper.. the proposed solution does not work, because it breaks tests and building apps.. Unfortunately, i was not able to find a proper solution (yet) that allows building the app, testing and refactoring in VSCode..

note that i also tried with a clean (!) and new installation with VSCode without any extensions in order to be sure it is not an issue with 3rd party packages!

johannesschobel avatar Jan 12 '21 11:01 johannesschobel

I really believe that the tsconfig.json references attribute is not correct here. See the official docs ( https://www.typescriptlang.org/docs/handbook/project-references.html ) for more details:

Referenced projects must have the new composite setting enabled. This setting is needed to ensure TypeScript can quickly determine where to find the outputs of the referenced project. Enabling the composite flag changes a few things:

  • The rootDir setting, if not explicitly set, defaults to the directory containing the tsconfig file
  • All implementation files must be matched by an include pattern or listed in the files array. If this constraint is violated, tsc will inform you which files weren’t specified
  • declaration must be turned on

The main tsconfig.base.json file, however, explicitly sets declaration to false, furthermore, the composite flag is missing in the referenced projects.

johannesschobel avatar Jan 12 '21 14:01 johannesschobel

ok.. i found a dirty workaround for my issue. I had an old workspace where everthing works as expected. I checked the differences between the those workspaces and found some changes in the tsconfig files, as highlighted below.

I did the following things:

  1. change every library tsconfig.json to this:
{
{
  "extends": "../../../tsconfig.base.json",
-  "files": [],
+  "include": ["**/*.ts"],
  "references": [
    { "path": "./tsconfig.lib.json" },
    { "path": "./tsconfig.spec.json" }
  ]
}
  1. change every application tsconfig.json to this:
{
  "extends": "../../tsconfig.base.json",
-  "files": [],
+  "include": ["**/*.ts"],
  "references": [
    { "path": "./tsconfig.app.json" },
    { "path": "./tsconfig.spec.json" }
  ]
}
  1. Restart VSCode and try to move a file

I don't know, what this workaround actually breaks, however, so far, it works better than the originally suggested approach! Maybe someone from Team Nrwl can take a look at this and suggest a better (?) solution? It would be very appreciated!

Thank you very much!

johannesschobel avatar Jan 12 '21 14:01 johannesschobel

I was able to reproduce this issue. This is odd indeed.

From what I understand, "Solution" tsconfig.json files allow tsconfig.json to delegate out to different tsconfigs. Editors, this means that instead of every file within a directory being type-checked by tsconfig.json, they can now be type-checked by the first referenced tsconfig. This is particularly useful where different files within a directory are built with different tsconfig files which can have different compilerOptions such as the types that are available etc.. Utilizing this allows the compilation to be safer and less error-prone because it is no longer possible use jest types such as describe in your runtime code. I believe this is a good trade-off to take because the applications we ship are more robust because of it.

Now because different files can use different tsconfig files for type-checking, I imagine VSCode is unable to "reuse" the same program? In theory, this was also possible before if you moved a file to outside the library though I guess I didn't expect it to work because I understand it's complex.

I am able to achieve this behavior "automatically" in a sequence of steps in certain conditions (I know, bear with me) via the following:

  1. Rename a file
  2. In files where things are imported from that file, remove the import
  3. Automatically import "Add all missing imports" as a fix for somewhere that isn't imported at the moment.
  4. VSCode will add the import to the new file.

So obviously, this doesn't work in all scenarios .e.g. when files are re-exported and it's not always able to add all missing imports but I think it shows that VSCode could handle those particular conditions.

Thus, I think this might actually be a bug in vs code so I suggest reporting it there.

Also FWIW, Webstorm is able to automatically update the imports when doing the same file move. Not to say that you should switch but to say that this editor functionality is theoretically possible.

FrozenPandaz avatar Jan 15 '21 16:01 FrozenPandaz

Dear @FrozenPandaz , damn, i am actually very glad you could reproduce this issue. I also played around with the solution file, however, just the solution file alone did not solve the issue for me.

I guess, WebStorm uses a different approach for finding / refactoring files than VSCode is.. However, due to the fact, that VSCode is free to use, and has nowadays a very large userbase, this issue should be addressed by nrwl.

Actually, i dont think that this is a bug in VSCode. The functionality was there in nrwl/nx 8 (or 9?) and it is a regression for later versions. Also, bootstrapping a plain nestjs application (without nrwl/nx) also works as intended..

All the best and thank you very (!) very much! Johannes

johannesschobel avatar Jan 18 '21 15:01 johannesschobel

This is a long shot, but I have noticed that in earlier version of nx, library included references used to be added to tsconfig.app.json:

{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "outDir": "../../dist/out-tsc",
    "types": []
  },
  "files": ["src/main.ts", "src/polyfills.ts"],
  "include": [
    "**/*.d.ts",
    "../../libs/your-lib/src/index.ts",
    "../../libs/your-other-lib/src/index.ts"
  ]
}

You see an example of this in the nrwl/nx-examples here

These references are no longer created with newer releases of nx, at least not for my workspace.

Maybe not having these includes causes VSC to no longer follow the dependency chain looking for import's to refactor?

christianacca avatar Jan 31 '21 10:01 christianacca

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs. If we missed this issue please reply to keep it active. Thanks for being a part of the Nx community! 🙏

github-actions[bot] avatar Mar 29 '21 13:03 github-actions[bot]

unstall this issue ;)

johannesschobel avatar Mar 29 '21 13:03 johannesschobel

it's a bug, so we keep it active

hiepxanh avatar Apr 24 '21 07:04 hiepxanh

I wrote up a blog post about how to setup typescript project references with NX. https://jakeginnivan.medium.com/using-typescript-project-references-in-nx-b3462b2fe6d4

JakeGinnivan avatar Jun 11 '21 07:06 JakeGinnivan

Still existing in Nx 12.x, any progress on this?

asbrum avatar Sep 06 '21 15:09 asbrum

Dear @FrozenPandaz ,

i know, you nx guys may be very busy because of your conference, but this issue is a real road block for larger projects. may you distribute this issue to someone who may be able to work on the latter? Maybe the solution provided by @JakeGinnivan is suitable and can be incorporated somehow!?

All the best and thanks for your awesome work! Johannes

johannesschobel avatar Sep 17 '21 06:09 johannesschobel

Dear @FrozenPandaz and whomever else, I just watched my 4 library, 3 app Nx monorepo fall apart in a couple of hours due to this nasty bug. As soon as it seems fixed, I get a ticker of new errors filing up my consoles. I'm going to have to attempt to take video of a section of interaction with it and try to pass it off as the POC i was to live demo for GM on Tuesday. Just slightly embarrassing fellas, not to mention my CTO might need to adjust my role to not working there. HUGE, show stopping, career smashing bug. Confused as to how it was even missed.

effinrich avatar Nov 14 '21 12:11 effinrich

Any update on this? So far this has not caused me any problems other than files being shown with errors in the file explorer in vscode, but this should definitely be fixed.

This is, based on the error message, incorrect usage of typescript 🤷🏻‍♂️

gioragutt avatar Dec 29 '21 23:12 gioragutt

It actually does cause problems when you're trying to get the typescript work in watch mode - it disrespects imported libs

jahglow avatar Jan 13 '22 13:01 jahglow

This issue still exists. Any news?

To note I am on the latest versions (13.3.1).

dandouglas avatar Feb 17 '22 12:02 dandouglas

I too am still seeing this issue

duro avatar Apr 10 '22 07:04 duro

same, any updates here?

SteffenAnders avatar May 13 '22 14:05 SteffenAnders

i think this issue is still present in v14.x, right? Anyone from the nrwl team eager to look into this issue? ;) That would be awesome!

johannesschobel avatar May 13 '22 18:05 johannesschobel

@FrozenPandaz Any update on this?

normtronics avatar Jun 15 '22 05:06 normtronics

Same here with a relatively new project

ymc9 avatar Jun 21 '22 02:06 ymc9

Still alive!

jedihacks avatar Aug 19 '22 11:08 jedihacks

Still alive!

jchamale avatar Sep 06 '22 17:09 jchamale

Latest update seemed to clear up the error in Nx generated apps and libs. In the case of a migrated CRA app, which I just assume will have wonky issues for its lifetime, the error persists. That is something of an improvement at least. That said the errors could simply return one day with no clear reason, which has happened on a couple of occasions.

effinrich avatar Sep 21 '22 07:09 effinrich

Yep, they reappeared in a brand new workspace. I mean...come on.

effinrich avatar Nov 01 '22 18:11 effinrich

Yeah, we've been getting these errors and agree that lack of support for Typescript project references is absolutely a show-stopper. The problem has forced us to evaluate other monorepos frameworks.

For all the virtues of NX, any large enterprise monorepo (which is where NRWL's revenues come from), is going to attempt to use typescript project references at some point. Microsoft is promoting this stuff so hard, it can't be ignored.

When our monorepo was less complex, this implementation of Typescript project references made NX dramatically faster.

It's painful to see NRWL drop the ball so badly on this one...

ngault avatar Dec 06 '22 21:12 ngault

I seem to be functioning by adding "composite": true to the compilerOptions in each library's tsconfig.lib.json. These files already have "declaration": true.

I run into trouble if I try to also add "composite": true to the library tsconfig.spec.json files.

UPDATE: I'm now able to compile without "composite": true. I had generated my libraries without the --buildable switch. Making them buildable eliminated the compilation problem.

jtlapp avatar Jan 24 '23 20:01 jtlapp

@juristr sorry for the direct mention, wanted to make sure this issue is known

gioragutt avatar Jan 24 '23 20:01 gioragutt

👍

kepek avatar Jan 31 '23 10:01 kepek

I'm now struggling with VSCode suddenly/periodically getting confused about the validity of the TypeScript configuration. It reports odd errors that go away -- and stay away -- when I reload VSCode.

I suspect it's moving .ts files from one directory to another within the source that messes things up, but I'm having trouble imagining the connection.

I may yet try @JakeGinnivan's solution for using TypeScript with NX.

jtlapp avatar Jan 31 '23 15:01 jtlapp