nrwl / nx

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

Cannot build library which depends on publishable lib in sub folder #2794

Closed MadeleineCodes closed 4 years ago

MadeleineCodes commented 4 years ago

Expected Behavior

It should be possible to build a publishable lib that depends on another publishable lib that resides in a sub folder.

Current Behavior

The issue I have is similar to https://github.com/nrwl/nx/issues/602 which is already fixed and working. However, if I have my libs in sub folders (e.g. libs/platform/my-lib) then it still fails with 'rootDir' is expected to contain all source files.

Failure Information (for bugs)

Steps to Reproduce

  1. I created a new nx angular workspace and added 2 publishable libs in the directory 'platform'.

    yarn create nx-workspace
    nx g lib core --directory platform --publishable --simple-module-name
    nx g lib common --directory platform --publishable --simple-module-name
  2. Created 1 class in each lib. The class in common uses the one from core like so:

import { Foo } from '@my-org/platform-core';

export class Bar {

  public run(): void {
    new Foo().run();
  }
}
  1. In addition I modified the tsconfig.json on root level from

    "@my-org/platform/core": ["libs/platform/core/src/index.ts"],
    "@my-org/platform/common": ["libs/platform/common/src/index.ts"]

    to

    "@my-org/platform-core": ["libs/platform/core/src/index.ts"],
    "@my-org/platform-common": ["libs/platform/common/src/index.ts"]

    (not sure why its created like this although in the package.json the libs are named "@my-org/platform-core" and "@my-org/platform-common") However, it does not make any difference. So even if I keep it like @my-org/platform/core (in tsconfig.json and the import in the class in common lib) the error is the same.

  2. Then run nx build platform-core (works!) nx build platform-common (fails)

See also: https://github.com/mrucelum/nx_publishable_lib

Context

@nrwl/angular : 9.2.1 @nrwl/cli : 9.2.1 @nrwl/cypress : 9.2.1 @nrwl/eslint-plugin-nx : Not Found @nrwl/express : Not Found @nrwl/jest : 9.2.1 @nrwl/linter : Not Found @nrwl/nest : Not Found @nrwl/next : Not Found @nrwl/node : Not Found @nrwl/react : Not Found @nrwl/schematics : Not Found @nrwl/tao : 9.2.1 @nrwl/web : Not Found @nrwl/workspace : 9.2.1 typescript : 3.8.3

Failure Logs

Compiling TypeScript sources through ngc
ERROR: libs/platform/core/src/index.ts:1:15 - error TS6059: File '~/nx_publishable_lib/libs/platform/core/src/lib/foo.ts' is not under 'rootDir' '~\nx_publishable_lib\libs\platform\common\src'. 'rootDir' is expected to contain all source files.

1 export * from './lib/foo';
                ~~~~~~~~~~~
libs/platform/common/src/lib/bar.ts:1:21 - error TS6059: File '~/nx_publishable_lib/libs/platform/core/src/index.ts' is not under 'rootDir' '~\nx_publishable_lib\libs\platform\common\src'. 'rootDir' is expected to contain all source files.

1 import { Foo } from '@my-org/platform-core';
                      ~~~~~~~~~~~~~~~~~~~~~~~

An unhandled exception occurred: libs/platform/core/src/index.ts:1:15 - error TS6059: File '~/nx_publishable_lib/libs/platform/core/src/lib/foo.ts' is not under 'rootDir' '~\nx_publishable_lib\libs\platform\common\src'. 'rootDir' is expected to contain all source files.

1 export * from './lib/foo';
                ~~~~~~~~~~~
libs/platform/common/src/lib/bar.ts:1:21 - error TS6059: File '~/nx_publishable_lib/libs/platform/core/src/index.ts' is not under 'rootDir' '~\nx_publishable_lib\libs\platform\common\src'. 'rootDir' is expected to contain all source files.

1 import { Foo } from '@my-org/platform-core';
juristr commented 4 years ago

@mrucelum Thx for reporting and for the detailed reproduction steps. Helps a lot to save time on our side πŸ™‚

I followed your steps and also cloned your repo and the build works for me. but only of course if I made the adjustments to the tsconfig.json you mentioned (In addition I modified the tsconfig.json on root level from)

The problem right now is the following: by default, we define the path mapping of libraries in tsconfig.json just following their path structure, meaning you end up with @my-org/platform/core as you've seen. That works fine as long as you don't have publishable libraries.

Publishable libraries (aka buildable libraries) are intended to be buildable on their own (and not as part of an app) & publishable to NPM for instance. That's also why they have their own package.json like

{
  "name": "@my-org/platform-core",
  "version": "0.0.1",
  "peerDependencies": {
    "@angular/common": "^9.1.0",
    "@angular/core": "^9.1.0",
    "tslib": "^1.10.0"
  }
}

A valid package name cannot have muliple / in there, basically a name like @my-org/platform/core is not allowed. The problem from that is that they are no more in sync now with what is being used in the default tsconfig mapping with nested libraries.

I'll give this a look. Meanwhile manually adjusting those libraries should work. Otherwise let me know.

MadeleineCodes commented 4 years ago

@juristr thanks you for the fast and detailed reply! And sorry for my late response... :/

I am not sure if I can follow you which manual adjustments I should do? My adjustments did not help (on my system at least :) ). I also did a fresh check out and install of my sample and I cannot make it run without errors.

I also tried to switching back to

"@my-org/platform/core": ["libs/platform/core/src/index.ts"],
"@my-org/platform/common": ["libs/platform/common/src/index.ts"]

in the tsconfig.json (and adapting also the imports in the usages). But that also does not work for me. I haven't found any solution which works for me so far. The only way to make it work for me is having all libraries directly beneath "libs" and in no subfolders - which is okay for now. But it would be very nice if it would work with sub directories. :)

Thanks again :)

manfredsteyer commented 4 years ago

Same here. It prevents me from using incremental compilation with: nx build my-app --with-deps.

Is there a solution on the horizon?

beeman commented 4 years ago

I'm seeing similar issues - happy to also provide a repo with steps if that helps.

georgiee commented 4 years ago

Hello folks, we have the same issue in our project. That's why I decided to create a demonstration project to help here. As far as I can see, the build error boils down to a wrong dependency graph calculation when nested folders are involved. Maybe some affected people can provide some more probes/reports? It looks like it is Windows only so far and I would like to confirm it or prove it wrong.

Issue: https://github.com/nrwl/nx/issues/2955 Repo: https://github.com/georgiee/nx-debug-nested-dep-graph

Thank you.

TimGeerts commented 4 years ago

I'm having the same issues, changing the tsconfig path syntax from / to - doesn't seem to help either, I keep getting "rootDir" errors.
Looking at the dep-graph, it's not even noticing that for instance my "libA" is dependant on "libCore" at all, it just shows them side by side.

georgiee commented 4 years ago

@TimGeerts can you provide your system specs? Is it Windows (Version? Node Version?) or OSX? You can see in my previous comment a linked issue with an extensive explanation of the issue. I now added the screenshot from it in this thread and it's exactly showing what is happening on your side. It would be interesting if you are on Windows as this would confirm our observations.

MadeleineCodes commented 4 years ago

My system specs: Windows 10 Enterprise Node 12.16.0

Maybe I can check whether my repo works on a Linux system in the afternoon.

georgiee commented 4 years ago

Hello @mrucelum, that system looks pretty familiar. Have you seen the linked issue?

https://github.com/nrwl/nx/issues/2955

There is an extensive analysis plus minimal example where we can perfectly reproduce the issue of the missing dependencies (on Windows while OSX is good).

Could you try out the minimal example and report the following:

You don't have to build or run a server β€” just compile the dep-graph. (as described in https://github.com/nrwl/nx/issues/2955)

git clone https://github.com/georgiee/nx-debug-nested-dep-graph
yarn install

# this always works because the folder structure is falt
git co scenario-flat
yarn nx dep-graph

# Nested libs folder structure. Only works on OSX and fails on OSX
git co scenario-nested
yarn nx dep-graph

Results in a gist on our side:

Untitled

TimGeerts commented 4 years ago

@georgiee , I'm on Windows 10 (Enterprise, v1709) indeed, node 12.15.0. I'll look into cloning the example repo and report back the graph output if you like!

edit I cloned the repo and ran the commands, the results are the same as in the bottom picture above (your post), so the "bad" windows 10 version where the nested (d) library is floating around in space like it's not attached to anything

MadeleineCodes commented 4 years ago

Hi, I can confirm that the issue seems to be a windows only issue. My own repo works fine when working on Linux (tested on pop os (ubuntu based)).

In addition I have the same behavior as @georgiee describes with his repo (https://github.com/georgiee/nx-debug-nested-dep-graph) under windows 10 enterprise:

flat flat nested nested

georgiee commented 4 years ago

Thank you @mrucelum, @TimGeerts & @jbjhjm. All of your reports confirmed that Windows 10 Enterprise causes the problem. I will try to reproduce it in a Windows 10 VM. I did not succeed with this before (it just worked before) but it was early to this debugging journey and I'm unsure what version and I used. In addition I had no repository that reproduced the problem so clearly at the time of testing.

If someone wants to try some debugging in parallel β€” I put some debugging hints in my comment here: https://github.com/nrwl/nx/issues/2955#issuecomment-628572471

Maybe some maintainer of the Nx Workspace recognize this is a true bug and join our discussion in the meantime.

benjamincharity commented 4 years ago

Chiming in here because I feel I am having a very similar issue on OSX.

I am working on migrating from an ng-packagr setup to NX and having issues with secondary entry points.

Of course we're never given time for quality of life improvements πŸ™ƒ, so making the move to NX is mostly happening on my own time and the impact to consumers needs to be as minimal as possible.

My goal

Move from a single published library that exposed two levels of entry points:

yarn add @terminus/ui
...
import { TsInputModule } from '@terminus/ui/input';
import { TsInputTestHelper } from '@terminus/ui/input/testing';
...etc

To multiple libs each with a secondary 'testing' endpoint (ala @angular/material):

yarn install @terminus/ui-input
...
import { TsInputModule } from '@terminus/ui-input';
import { TsInputTestHelper } from '@terminus/ui-input/testing';
...etc

The issue

When I first moved just the components over without any secondary testing inputs, all worked well. I could build all components or some with the with-deps flag.

Once I added the secondary entry points for testing it all fell apart. Just the addition of the entry points isn't causing the issue, but I believe NX is getting lost in circular dependencies. I'm just not sure why ng-packagr was ok with it but NX fails.

Example of what causes failure:

  1. @terminus/ui-input exports an input that uses a form-field
  2. @terminus/ui-input/testing exports some test components and helper functions
    • These test files require items from @terminus/ui-form-field
  3. @terminus/ui-form-field exports a form-field wrapper used by several form components
  4. @terminus/ui-form-field/testing exports some test components and helper functions
    • These test files require items from @terminus/ui-input

Stepping back it seems logical that this is really just two packages so it's causing circular dependencies. BUT ng-packagr seems to handle this just fine so I'm wondering what the difference is here. I've seen some other issues where people have fallen back to using ng-packagr manually, but I'd really like to leverage NX fully.

Failures

When using --with-deps flag:

RangeError: Maximum call stack size exceeded

Screen Shot 2020-05-22 at 8 06 24 AM

When using nx run-many --all --target build

Screen Shot 2020-05-22 at 8 07 35 AM

(Running again with the only-failed flag has identical output)

Setup / Reproduction

Here is a branch with a reproduction: https://github.com/GetTerminus/terminus-oss/tree/nx-repro

(

➜  terminus-oss git:(master) nx report

>  NX  Report complete - copy this into the issue template

  @nrwl/angular : 9.3.0
  @nrwl/cli : 9.3.0
  @nrwl/cypress : 9.3.0
  @nrwl/eslint-plugin-nx : Not Found
  @nrwl/express : Not Found
  @nrwl/jest : 9.3.0
  @nrwl/linter : 9.3.0
  @nrwl/nest : Not Found
  @nrwl/next : Not Found
  @nrwl/node : 9.3.0
  @nrwl/react : Not Found
  @nrwl/schematics : Not Found
  @nrwl/tao : 9.3.0
  @nrwl/web : Not Found
  @nrwl/workspace : 9.3.0
  typescript : 3.8.3

➜  terminus-oss git:(master) ng --version

     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / β–³ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/

Angular CLI: 9.1.6
Node: 12.10.0
OS: darwin x64

Angular: 9.1.0
... animations, common, compiler, core, forms, platform-browser
... platform-browser-dynamic, router
Ivy Workspace: No

Package                            Version
------------------------------------------------------------
@angular-devkit/architect          0.901.6
@angular-devkit/build-angular      0.901.6
@angular-devkit/build-ng-packagr   0.901.6
@angular-devkit/build-optimizer    0.901.6
@angular-devkit/build-webpack      0.901.6
@angular-devkit/core               9.1.6
@angular-devkit/schematics         9.1.6
@angular/cdk                       9.2.3
@angular/cli                       9.1.6
@angular/compiler-cli              9.1.8
@angular/flex-layout               9.0.0-beta.29
@angular/language-service          9.1.8
@angular/material                  9.2.3
@ngtools/webpack                   9.1.6
@schematics/angular                9.1.6
@schematics/update                 0.901.6
ng-packagr                         9.1.5
rxjs                               6.5.5
typescript                         3.8.3
webpack                            4.42.0

(btw very impressed with NX so far - really hopeful we can make the full switch ❀️ )

-edit

Possibly related:

benjamincharity commented 4 years ago

I've done a bit more testing today and think it has less to do with secondary endpoints as it does NX just not understanding how to manage the circular deps.

From my original starting point in the previous message I created a branch and removed all secondary testing endpoints - just relying on tree shaking so consumers don't ship testing code.

Just removing the secondary endpoints didn't solve the issue. The issue seems to be libraries whose tests depend on other libraries.

Extrapolation from previous example:

  1. FormField library exports a component <form-field>
  2. Input uses the form-field in their template so when a user adds <input></input> the dom is actually <form-field><input></input></form-field>
  3. Input specs rely on FormField since it is a part of the Input component and the test verifies interaction between them.
  4. FormField doesn't directly rely on Input, but the FormField specs do (some would say FormField should only test using itself, but we have found that even with 100% unit testing, bugs would slip through based on how the templates communicated).
  5. So now NX considers FormField a dependency of Input and Input a dependency of FormField.

On the branch attempt-merging-testing-into-modules I have a) removed all secondary entry points b) commented out any specs that import another module causing circular issues.


All in all this doesn't seem like a crazy solution.. but again, since ng-packagr seemed to handle it well in our previous repo I am curious if there is any way around this that doesn't involve so many changes to our structure.

If I imagine that the two packages (FormField and Input) were existing, published packages it doesn't seem like a big issue that the FormField library pulls in Input as a devDependency for running specs.

--

Open to any insight here or other possible paths from those with much more NX knowledge than me (which is probably most of you πŸ˜‚ )

georgiee commented 4 years ago

@benjamincharity Besides using scope you describe a very different error. You should create a separate issue if you want a chance of being recognized. Don't forget to provide a minimal example that reproduces the error.

agroupp commented 4 years ago

Still facing this issue on version 9.4.4. All tests pass and build fails.

TimGeerts commented 4 years ago

Has there been any progress on this case (libraries depending on other libraries not being able to build on windows 10)? We want to start restructuring our libraries and it would be so much easier (and more correct) if we were able to depend publishable libraries on other publishable libraries.

juristr commented 4 years ago

We're on it, sorry for the delay.

juristr commented 4 years ago

Hey all, sorry for the long wait 😞 .

So we basically just merged #2840 and it should have been already released with v10.0.3. Just upgrading won't work as we didn't do any migration because that would have been difficult & probably suboptimal.

So what changed?

So what did I add in the PR. Basically we're currently trying to optimize the incremental build story & the PR is a first step in that direction. So the PR now clearly distinguishes between --publishable and --buildable. While they were aliases before, now there's a difference.

--publishable now requires you to pass in a required --importPath param, which should be the npm name you plan to use when releasing the package (e.g. @myorg/my-awesome-package or simply my-awesome-package). We require that --importPath explicitly as we didn't want to do any guesswork here. That importPath is what the TSConfig path mappings will use & what will be in your package.json

--buildable instead doesn't do any TSConfig path manipulation or require any additional work. It is basically kind of the --publishable from Nx v9. Why would you need a buildable library? For incremental building. See our docs for more info and an example repo.

How can I fix my workspace?

The best is to generate a new Nx v10 workspace, create a similar scenario you're having in your app & have a look at the differences. It should mostly be the TSConfig path mappings & package.json names.


If you have questions, or if there are still issues, please let me/us know and open a new issue :)

Splaktar commented 3 years ago

The problem right now is the following: by default, we define the path mapping of libraries in tsconfig.json just following their path structure, meaning you end up with @my-org/platform/core as you've seen. That works fine as long as you don't have publishable libraries.

A valid package name cannot have multiple / in there, basically a name like @my-org/platform/core is not allowed. The problem from that is that they are no more in sync now with what is being used in the default tsconfig mapping with nested libraries.

@juristr is this still the guidance for buildable libraries?

Or was that changed? I'm asking since all of the examples in nx-incremental-large-repo like https://github.com/nrwl/nx-incremental-large-repo/blob/master/libs/app0/lib0/lib0/package.json#L2 are like "name": "@largerepo/app0/lib0/lib0".

juristr commented 3 years ago

@Splaktar exactly, that is no more necessary. Publishable libraries need to have a correct path mapping since they are made for publishing and therefore need to follow those rules. In fact, when you generate a new publishable lib, you have to explicitly provide an --importPath.

Buildable libs don't have that restriction.

maxime1992 commented 3 years ago

I just spent 2 solid hours debugging the error

error TS6059: File '/some/path/index.ts' is not under 'rootDir' '/some/other/path/src'. 'rootDir' is expected to contain all source files.

I finally understood where it came from :exploding_head:.

I generated a bunch of libraries, some of them were in nested folders.

I messed up and didn't the name of the library would be concatenated so I generated something like this

libs
  folder1
    folder1-name1
    folder1-name2

and couldn't bother rename them just yet (I'm working on a huge and 100% independent task so a long lived branch isn't an issue here and wanted to do that at the end).

At some point, I ended up renaming the libs everywhere in angular.json and nx.json because their names were

folder1-folder1-name1
folder1-folder1-name2

So I did rename them to

folder1-name1
folder1-name2

Same for the TS mapping as it was annoying.

So I renamed them everywhere. Or at least that's what I thought. But I completely forgot about the fact that buildable libs have their own name written in their package.json and the name was still based on the old one which I didn't find with the global text search when looking for folder1-folder1-name1 as in the package.json it was { "name": "@corp/folder1/folder1-name1" }.

Taking the time to share in case anyone else does that or simply for my future self. I'm sure I'll re-read this thread desperately in a few years :facepalm:

Splaktar commented 3 years ago

error TS6059: File '/some/path/index.ts' is not under 'rootDir' '/some/other/path/src'. 'rootDir' is expected to contain all source files.

@maxime1992 I ran into the same problem and it was a nightmare (multiple days) to debug. I finally just changed the libs to no longer nest and it finally fixed it.

For clarity, your fix was to just change the name in the package.json of the nested or parent lib?

maxime1992 commented 3 years ago

@Splaktar

it was a nightmare

Agree... Really no clue of what was going on

(multiple days) to debug

Uch. I was lucky enough to find out where it was coming from in ~2 hours for this one :four_leaf_clover:

I finally just changed the libs to no longer nest and it finally fixed it.

   
We already had at least 3 nested libs so I knew they were working just fine and something else was wrong image

For clarity, your fix was to just change the name in the package.json of the nested or parent lib?

In my case I had indeed to only change the name in package.json of the library to match the name defined as a custom TS path that I was using in all my imports. I believe the reason this is working in dev mode is probably because it just doesn't use the incremental compilation of nx with the --with-deps options that we've got for the build command. Probably just reads from the tsconfig.base.json file which defines the name of the imports through the paths property. But then, when building the app with --with-deps it does that by block of work and build intermediate libraries. When trying to link an import to another library, it doesn't refer to the tsconfig.base.json anymore, it just looks into the dist folder for a library with this package name (sort of what it does when importing a lib from node_nodules) but in the dist folder. At least that's my assumption.

On NX side, I get a tiny check to make sure that all the package.json in libraries have a name which exists in the tsconfig.base.json in the paths property may be useful. If not, at least display a warning saying hey this is probably not gonna work

guiseek commented 3 years ago

I fixed exact issue by adding to each library's tsconfig.lib.json one line:

"paths": { "@yourscope/*": ["dist/libs/*"] }

This will allow typescript to load your libs instead of compiling them. Also make sure you build them in order they depend on each other.

simondotm commented 3 years ago

I fixed exact issue by adding to each library's tsconfig.lib.json one line:

"paths": { "@yourscope/*": ["dist/libs/*"] }

This will allow typescript to load your libs instead of compiling them. Also make sure you build them in order they depend on each other.

At first glance this looks like a clever solution, but isn't it going to cause developer havok as their IDE is parsing "built" code that is potentially out of date from the library source code they might be working on?

guiseek commented 3 years ago

At first glance this looks like a clever solution, but isn't it going to cause developer havok as their IDE is parsing "built" code that is potentially out of date from the library source code they might be working on?

the problem I had was when the repo was cloned and failed to run the server, as it looked for the library in the dist directory and there was none. so now i put your build on the postinstall hook. but I understood what you said and it really is not the best solution, just a workaround to move on. what is certain is that all paths are configured in tsconfig.base.json. as usual

guiseek commented 3 years ago

the problem with me was due to using @scope/prefix-name in lib package.json and @scope/prefix/name in tsconfig.base.json

I renamed all lines to @scope/prefix-name and added the other lib as a dependency, the error stopped

oh, another thing I did was use the @nrwl/web:package builder instead of @nrwl/workspace:tsc

guiseek commented 3 years ago

@simondotm see my comment above, this way I was able to remove the tsconfig path from my lib

simondotm commented 3 years ago

oh, another thing I did was use the @nrwl/web:package builder instead of @nrwl/workspace:tsc

Ah ok, that would make sense, since 1) yes you need to use npm compatible package names in the tsconfigs and 2) I believe that builder generates its own internal temporary tsconfig that remaps paths for library dependencies from the tsconfig.base.json to point to dist - see here (took me a while to figure this out on my own project). Glad you got it going.

Fasani commented 3 years ago

I struggled a lot with this over the last few days trying many many things, I have some core helpers and types which I wanted to include in a publishable UMD app. In the end, for me, it was very simple to bypass the 'rootDir' is expected to contain all source files. error.

Maybe this helps some of you, I added the helper and the types like normal.

import { colorSchemesFindHexById } from '@org-name/shared/helpers/color';
import {
    WidgetInitOptions,
    WidgetCommunicationData,
    InitProps,
} from '@org-name/shared';

Then I have added the exact locations of where these things can be found to the tsconfig.json in the app:

"compilerOptions": {
    ...
    "rootDirs": [
        "../../libs/shared/src/helpers/color",
        "../../libs/shared/src/models"
    ]
},
jshaw-cm commented 3 years ago

nx version: 12

I'm not sure if anyone is still having this issue, but I ran into the exact same error ('rootDir' is expected to contain all source files.)

I was trying to import a publishable lib into another publishable lib (using node:lib --buildable --publishable)

I spent about 3 hours on it and decided to remove those packages from my package.json (not sure why I did that at one point) and install node modules without them to build again.

After that the issue was resolved. Screen Shot 2021-06-29 at 3 55 43 PM

Not sure why anyone would install those packages in your root package.json of an nx workspace, but if you do that it could be an issue that looks exactly like this.

Here's the error, just to confirm it's the same error.

Screen Shot 2021-06-29 at 3 59 24 PM

smasala commented 2 years ago

For the record: this error will still appear under windows if you are referencing code that hasn't been built into the dist folder correctly. For example referencing lib level generated code like a prisma client which isn't automatically copied to the dist folder will cause this somewhat unhelpful error.

github-actions[bot] commented 1 year ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.