nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.63k stars 2.36k forks source link

Issue in generated tsconfig files for apps & libs #4508

Open johannesschobel opened 3 years ago

johannesschobel commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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" }
  ]
}

2) change every application tsconfig.json to this:

{
  "extends": "../../tsconfig.base.json",
-  "files": [],
+  "include": ["**/*.ts"],
  "references": [
    { "path": "./tsconfig.app.json" },
    { "path": "./tsconfig.spec.json" }
  ]
}

3) 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!

FrozenPandaz commented 3 years ago

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.

johannesschobel commented 3 years ago

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

christianacca commented 3 years ago

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?

github-actions[bot] commented 3 years ago

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! 🙏

johannesschobel commented 3 years ago

unstall this issue ;)

hiepxanh commented 3 years ago

it's a bug, so we keep it active

JakeGinnivan commented 3 years ago

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

asbrum commented 3 years ago

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

johannesschobel commented 3 years ago

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

effinrich commented 2 years ago

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.

gioragutt commented 2 years ago

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 🤷🏻‍♂️

jahglow commented 2 years ago

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

dandouglas commented 2 years ago

This issue still exists. Any news?

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

duro commented 2 years ago

I too am still seeing this issue

SteffenAnders commented 2 years ago

same, any updates here?

johannesschobel commented 2 years ago

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!

normtronics commented 2 years ago

@FrozenPandaz Any update on this?

ymc9 commented 2 years ago

Same here with a relatively new project

jedihacks commented 2 years ago

Still alive!

jchamale commented 2 years ago

Still alive!

effinrich commented 2 years ago

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 commented 2 years ago

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

ngault commented 1 year ago

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...

jtlapp commented 1 year ago

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.

gioragutt commented 1 year ago

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

kepek commented 1 year ago

👍

jtlapp commented 1 year ago

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.

binaryartifex commented 1 year ago

yep. can confirm its still persisting in the very latest nx. create a new nextjs app, slap a storybook configuration into the application, by default nx will generate a tsconfig.storybook.json. give it a hot minute and soon enough, the apps tsconfig.json will start bitching that the following reference ->

  "references": [
    {
      "path": "./tsconfig.storybook.json"
    }
  ]

must have setting "composite": true. basically anything that gets added to the references array will eventually trigger this error. everytime. its just a matter of time.

joerobot commented 1 year ago

+1, I just did what @binaryartifex mentioned and ended up here 🙃

cody-rountree-rakuten commented 1 year ago

I have the same experience. In fact, when I try to type check with tsc --noEmit -p apps/app/tsconfig.json, I receive failures saying that the referenced tsconfig in apps/app/cypress/tsconfig.json needs to have "composite" to true. I double checked that the nx config generators do not add the composite value by default. However, if I add the settings that the typescript engine wants, then VSCode has trouble resolving my imports in the files included in the composite tsconfig.json

StanislavPonomarenko commented 1 year ago

got same issue on new nx workspace, react with storybook Referenced project '/libs//tsconfig.lib.json' must have setting "composite": true

quincarter commented 1 year ago

also getting this on a repo with a newly generated plugin directory in nx 16.9.x

ngault commented 1 year ago

@StanislavPonomarenko yes, FWIW, our problem was also with (nx + react + storybook). This is an incredibly hard problem to work around, and grows disproportionately with the size of the codebase.

It's obviously a deep bug in nx, otherwise the core team wouldn't have ignored the issue and users' attempts to help for the past 33 months.

After 5 years of nx and 2 years of battling this bug, this week our team gave up and migrate moved to Turborepo, where Typescript "composite" references work like they're supposed to. VS Code is suddenly blazingly fast. 7 developers are shouting "hallelujah".

Sadly, the NX team seems to have fallen in love with adding features at the expense of fixing critical problems. A bad sign given the battle ahead.

miluoshi commented 1 year ago

@ngault care to share what your tsconfig files setup looks like now with turborepo in your codebase?

jtlapp commented 1 year ago

I just want to point out that Nx will work with any monorepo that's compatible with Turborepo -- and in my opinion, with better performance and cleaner output.

I know because I started with Nx, got fed up with it, switched to Turborepo, go that working, and then accidentally discovered that Nx generally runs better on a Turborepo-compatible monorepo than Turborepo does. That's because, while there is such thing as an Nx-specific monorepo (the "integrated monorepo"), there is no such thing as a Turborepo-specific monorepo (this is just a generic monorepo).

You only have to deal with Nx/TypeScript incompatibilities when using Nx's "integrated monorepo" setup. If you instead install a "package-based monorepo," you've got yourself a repo that works equally well with Turborepo and Nx, because you're no longer relying on Nx to wrap everything to get it working with their proprietary monorepo.

Please see my article on sharing TypeScript via Nx and Turborepo monorepos. I wrote it to help save others the pain I went through trying to get things working.

svallory commented 1 year ago

Sadly, the NX team seems to have fallen in love with adding features at the expense of fixing critical problems. A bad sign given the battle ahead.

I completely agree. I've used Nx in our company before, we moved out of it because of issues like this one. This is actually the THIRD time I'm trying to setup Nx, which promises to be simple, to the point of questioning people that say otherwise in a YT video. Once again, I'm about to give up because Nx will simply not work and the lack of documentation is astounding.

Sure, the docs are big, have a lot of pages and seem comprehensive. Until you start asking:

I'm deeply saddened by what's been happening with Nx. It has so much promise! But while this issue with a very fundamental issue has been a thorn for more than a year, in Nx 17 we get an update to the project inference API. It's maddening.

I sent a message in discord about this issue and #19028 and got completely ignored: https://discord.com/channels/1143497901675401286/1143540600252153946/1165002803102629978

Maybe with more people re-sharing the issue in general it will at least get a response from someone at @nrwl

ping: @vsavkin @FrozenPandaz @jaysoo or @juristr

algoflows commented 11 months ago

I am also fully stuck at the moment, composite true errors, and being unable to import libs into apps.... struggling to work with NX at the moment in a big way.

techpeace commented 11 months ago

If you have a JavaScript NX project and just want these problems to vanish from your VS Code problems panel, you can drop the offending lines from your config files:

Regardless whether you use JavaScript or TypeScript, you will have a tsconfig.base.json file at the root of the workspace. It's not used to build the applications and libraries in the workspace. It's only used to improve the editor experience.

svallory commented 11 months ago

If you have a JavaScript NX project and just want these problems to vanish from your VS Code problems panel, you can drop the offending lines from your config files:

Regardless whether you use JavaScript or TypeScript, you will have a tsconfig.base.json file at the root of the workspace. It's not used to build the applications and libraries in the workspace. It's only used to improve the editor experience.

That makes using nx even more confusing. And I believe that info about tsconfig having no effect should be in a really big warning in the docs. I've never noticed that and I've known Nx for years.

Btw, I'm not even sure that's true since changing things in the tsconfig does seem to affect the build. Especially regarding paths.

How can the TS compilation be customized then if not via the tsconfig file?

mkhib commented 11 months ago

This issue has been open for almost 3 years now! I have encountered this problem lately and now I see there is no solution for it! Amazing!

svallory commented 10 months ago

This issue has been open for almost 3 years now! I have encountered this problem lately and now I see there is no solution for it! Amazing!

We dropped Nx because we have soooo many problems like this it was making us much less productive. Since then things have been running smoothly. Ever since Nx became a feature collector, it's been a nightmare. NOTHING ever works out-of-the-box in Nx. The last straw came when I learned that Nx does NOT use the tsconfis to build the app.

Since no one from Nx team will even look at this, I'll leave my recommendation: I just start trying moonrepo and the experience has been awesome!

Btw, I'm in no way affiliated with moonrepo, just an angry Nx ex-customer who have lost weeks fixing broken nx setups

adelbenhamadi commented 8 months ago

Weirdly i have this experience with nx after creating a testing rmonorepo: i added some library under folder named libs and suddenly this nasty error appeared: "Referenced project '/home/adil/nx/ng-netflix-repos/libs/core/tsconfig.lib.json' must have setting "composite": true" and after a while when i was trying to solve the problem i noticed that when i modify "includes:[]" in libs/core/tsconfig.json to something like ""include": ["src/*/.ts"]" , then save and then undo the changes and save again : the error is gone. That's weird, that's how i solved the problem, i hope it will not appear again.

svallory commented 8 months ago

Weirdly i have this experience with nx after creating a testing rmonorepo: i added some library under folder named libs and suddenly this nasty error appeared: "Referenced project '/home/adil/nx/ng-netflix-repos/libs/core/tsconfig.lib.json' must have setting "composite": true" and after a while when i was trying to solve the problem i noticed that when i modify "includes:[]" in libs/core/tsconfig.json to something like ""include": ["src/*/.ts"]" , then save and then undo the changes and save again : the error is gone. That's weird, that's how i solved the problem, i hope it will not appear again.

That's probably just the language server taking a while to detect the issue again, or getting into a weird state. Check if closing the IDE and opening it again won't make the error reappear

adelbenhamadi commented 8 months ago

I'm not giving a solution, i just mentioned that accidentally i get rid of the problem: of course that's a bug. And for your question i's gone, it does not reappear

svallory commented 8 months ago

I'm not giving a solution, i just mentioned that accidentally i get rid of the problem: of course that's a bug. And for your question i's gone, it does not reappear

Sorry if I sounded rude. I know you were not offering a solution. And I'm glad to hear it didn't come back. :)

jensbodal commented 8 months ago

I'm not giving a solution, i just mentioned that accidentally i get rid of the problem: of course that's a bug. And for your question i's gone, it does not reappear

Hopefully it stays that way. I've tried looking into this a few times and have managed to "fix" it as far as I knew, but eventually a certain condition of opened files would cause the issue to reappear.

Would you be able to push your testing monorepo somewhere so that the setup can be seen?

svallory commented 8 months ago

I'm not giving a solution, i just mentioned that accidentally i get rid of the problem: of course that's a bug. And for your question i's gone, it does not reappear

Hopefully it stays that way. I've tried looking into this a few times and have managed to "fix" it as far as I knew, but eventually a certain condition of opened files would cause the issue to reappear.

Would you be able to push your testing monorepo somewhere so that the setup can be seen?

Sorry, but I've moved away from nx to moon (at least until they start handling typescript properly). Moonrepo docs even explains how and why you should use TS project references.