Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AOT/IVY/9.0.0-rc.0] Unable to compile application with imports from non-compiled library in monorepo. #33562

Closed
JonWallsten opened this issue Nov 4, 2019 · 9 comments

Comments

@JonWallsten
Copy link
Contributor

@JonWallsten JonWallsten commented Nov 4, 2019

馃悶 bug report

Affected Package

The issue is caused by package @angular/compiler(?)

Is this a regression?

No

Description

We're unable to compile (AOT/IVY) our application with imports from a non-compiled library in monorepo. I'm met with ERROR in Unable to write a reference to...

馃敩 Minimal Reproduction

Reproduction: https://github.com/JonWallsten/monorepo-repro
#install deps
npm run full-install

#start dev server
npm run dev

#go to browser
localhost:8888

馃敟 Exception or Error


ERROR in Unable to write a reference to MaxCharactersPipe in packages\web-app-angular\node_modules\web-lib-angular\src\pipes\max-characters.pipe.ts from C:/Users/me/repo/monorepo-repro/packages/web-app-angular/node_modules/web-lib-angular/src/pipes/pipes.module.ts

馃實 Your Environment

Angular Version:


Angular CLI: 9.0.0-rc.0
Node: 10.16.3
OS: win32 x64
Angular: 9.0.0-rc.0
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, material, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.0-rc.0 (cli-only)
@angular-devkit/build-optimizer   0.900.0-rc.0
@angular-devkit/core              9.0.0-rc.0 (cli-only)
@angular-devkit/schematics        9.0.0-rc.0 (cli-only)
@angular/http                     8.0.0-beta.10
@ngtools/webpack                  9.0.0-rc.0
@schematics/angular               9.0.0-rc.0
@schematics/update                0.900.0-rc.0 (cli-only)
rxjs                              6.5.3
typescript                        3.6.4

Anything else relevant?
We're using latest webpack and AngularCompilerPlugin from @ngtools/webpack to compile our application.

Relates to: #33482 and #29361

@saithis

This comment has been minimized.

Copy link

@saithis saithis commented Nov 4, 2019

I found this post, that helped me: https://dev.to/negue/angular-ivy-overcome-the-first-obstacles-400n

Just add
"rootDir": ".",
to your tsconfig compilerOptions and it works again.

@gnomeontherun

This comment has been minimized.

Copy link
Contributor

@gnomeontherun gnomeontherun commented Nov 4, 2019

Please also see https://github.com/gnomeontherun/monorepo-library-errors for a pure CLI based demo with clear steps to get from a new project to failure points.

@JonWallsten

This comment has been minimized.

Copy link
Contributor Author

@JonWallsten JonWallsten commented Nov 4, 2019

@saithis: Unfortunately that did not help. Still the same error.

@ngbot ngbot bot added this to the needsTriage milestone Nov 4, 2019
@atscott atscott added the comp: ivy label Nov 4, 2019
@alxhub

This comment has been minimized.

Copy link
Contributor

@alxhub alxhub commented Nov 6, 2019

Hello @JonWallsten,

The configuration here seems really confused and unnecessarily complex to me.

  • web-app-angular attempts to depend on web-lib-angular in 3 different ways:
  • via a path mapping in its tsconfig.json
  • by including the library sources directly into its compilation in tsconfig.json
  • by installing the raw library sources as into its node_modules via a dependency in package.json
  • there is a path mapping for @angular/* into node_modules/@angular/*, which is unnecessary as TypeScript should already know how to resolve Angular via node module resolution.

  • each package (app and lib) has its own package.json and its own node_modules. I would expect node_modules to be shared across the monorepo.

In general, a lot of configuration goes into making a monorepo work correctly. It's slightly more complicated in Ivy due to the need for each package's compilation to depend on the compiled output of its dependencies, not the sources.

I'm working on a doc for our docs site which describes a proper monorepo setup. Feel free to look through the prototype doc and let me know if you can follow it to set up a compilation in your monorepo. I'll be continuing to add new info to the doc over time.

@JonWallsten

This comment has been minimized.

Copy link
Contributor Author

@JonWallsten JonWallsten commented Nov 6, 2019

(note that some of the things here only concerns our real monorepo, not the repro one)

@alxhub: Thanks for taking the to looking into this! There are a few reasons why things are what they are if I'm not mistaken. It mostly has to do with how VSCode is doing things and then some with the current state of our application. We have a monorepo with both AngularJs and Angular due to legacy applications. This is the reason why most of the depenedecies are in the monorepo root and some are in the application. But I could try to move the them into the root anyway if that would solve this.

The reason for the "paths" thing is because we got the deep imports in some cases when using autocomplete/intellisense to import modules from angular. It had to do with the symlinking in our monorepo. It also has to do with declaration source maps, to allow us to Ctrl+click to any source file in our code from any package in the monorepo (we ended up in node_modules/@monorepo/package instead of the real path and got conflicts with our own files). All the strange things you see is probably connected to that. We had a lot of issues the symlinked local packages without all these things. Although I haven't touched this since I got everything to work more than a yeah ago. I can try to remove some "unnecessary" stuff and see if it still works.

The include of the library source was probably because it didn't end up in the compiler if I didn't include it.

So even if things looks weird everything is there for a specific reason to solve an annoying problem for the our devs.

Would this be solved if I compiled the lib instead of importing the sources directly? I can easily add a simple webpack config to compile it instead if that's needed.

After reading through your guide there are a few things that isn't as easy as I would like. A single top-level tsconfig is not possible since our packages requires differrent tsconfig settings. For example our new apps/lib uses "strictNullChecks" but to enable that for our old libs would takes weeks of work. This is causing an issue with the "paths" in the root level tsconfig since they will be inherited by the app/lib's tsconfig, and I guess the path will no longer be correct.

Edit: I'm making some progress. I basically have to redo everything. But I really appreciate your pointers. I didn't know a lot of it was redundant. And it was in the "Don't fix it if it isn't broken"-state.

Edit 2: I've succesfully built the project with the new settings per your suggestion. I'm using a single tsconfig in the root with paths for each packages. Each lib/app uses a tsconfig.build.json for specific build settings. There are however a few annoying things that needs to be sorted out for this to work. And they relates to the first things I wrote.

  1. The declaration files no longer ends up where they should. I think it related to that rootDir and baseUrl is removed from each package's tsconfig. I fixed this with webpack copy plugin so it's not a big deal.
  2. Imports from other packages nog longer works as it should (this is what I was afraid of). I now have two copies of each exported thing. One is pointing to the source, one is pointing to the dist typings and when I import it I don't get the package name as src but the realtive path in the monorepo. How did you guys solve this? (I guess VSCode would need some info to understand that sibling packages are their own module?)
    image

I checked the settings and found this:
image

So it's a little bit better now, but still not what it should be:
image

@alxhub

This comment has been minimized.

Copy link
Contributor

@alxhub alxhub commented Nov 6, 2019

(note that some of the things here only concerns our real monorepo, not the repro one)

Just a note, if you have some time and could make the repro more faithful to the original repo, that would be super helpful for me as I put together documentation to guide developers on how to set these kinds of things up. In particular I've found npm link to be particularly problematic when working in setups like this.

So even if things looks weird everything is there for a specific reason to solve an annoying problem for the our devs.

Oh, for sure. But often a better foundation can greatly reduce the amount of workarounds required to set things up correctly.

Would this be solved if I compiled the lib instead of importing the sources directly? I can easily add a simple webpack config to compile it instead if that's needed.

Generally, you should have one build per "package", and depend on the compilation output of any dependencies when building each package. The goal of the split editor/build tsconfigs is to allow the editor to see a dependency on sources (which makes cross-project "go to declaration" work well) while the compile of a package depends on the compiled version of each dependency. This also allows the editor to navigate through the whole repo without first building all the library packages, which is a drawback of plain TS project references.

With this setup, declaration source maps will be unnecessary.

After reading through your guide there are a few things that isn't as easy as I would like. A single top-level tsconfig is not possible since our packages requires differrent tsconfig settings. For example our new apps/lib uses "strictNullChecks" but to enable that for our old libs would takes weeks of work. This is causing an issue with the "paths" in the root level tsconfig since they will be inherited by the app/lib's tsconfig, and I guess the path will no longer be correct.

It's fine to have a separate editor configuration per package, which extends from the top level one and sets any package-specific options.baseUrl should be set to "." in the top-level one and be left unchanged elsewhere.

The declaration files no longer ends up where they should. I think it related to that rootDir and baseUrl is removed from each package's tsconfig.

I'm not sure what's going on here. Without further rearrangement of files, TS will always emit declaration files immediately next to the source files, which is what we want.

Imports from other packages nog longer works as it should (this is what I was afraid of). I now have two copies of each exported thing. One is pointing to the source, one is pointing to the dist typings and when I import it I don't get the package name as src but the realtive path in the monorepo. How did you guys solve this?

There are two ways TS can learn about a file.

  1. Having the file directly included in the sources of a combination.

This can happen because it's listed in files or includes of a tsconfig. Your first example was doing this by listing "../web-lib-angular/src/**/*.ts" in the includes of the web-app-angular tsconfig.

  1. Being imported by a file which is already in the project.

To get the experience in the IDE that we want, we need our editor to not include the dist files in the project. If they do get vacuumed into the tsconfig, then TS will discover two ways to import from them - directly (mechanism 1 above), and via the path mapped imports (when it encounters them - mechanism 2 above).

The easiest way to do this is to set outDir: "dist" in the top-level editor configuration. Because outDir is automatically excluded from the project via mechanism 1, TS will only learn about the existence of these files via the imports that we want it to use. Does that make sense?

Again, I would really love to understand your requirements for symlinking packages. All of my experiences with them have been profoundly negative, and I've developed the opinion that they should be avoided in almost all circumstances.

@JonWallsten

This comment has been minimized.

Copy link
Contributor Author

@JonWallsten JonWallsten commented Nov 7, 2019

Sure! I'll prepare two repos, one with our existing way of doing thing, and then one with the new implementation based on your pointer, docs and Angular's repo. Then you can see how it did work, and how it works now. Although I will make them private and add you as a collaborator so I don't have to spend more than then necessary on renaming and clearing out any traces to our project. But I will remove all real code and just keep a few generic imports to show what kind of dependencies we have. When I done we can look at annoying stuff with the imports and such.

@JonWallsten

This comment has been minimized.

Copy link
Contributor Author

@JonWallsten JonWallsten commented Nov 7, 2019

@alxhub
You should be able to access these repos now:
https://github.com/JonWallsten/monorepo-new (Angular 9, IVY, new config)
https://github.com/JonWallsten/monorepo-old (Angular 8, old config)
https://github.com/JonWallsten/monorepo-old/tree/ivy (Angular 9, IVY, oldconfig)

However, with the new way of using imports for Angular Material I get this new error I didn't get yesterday. Not sure if it's part of my strip or if it has something to do with rc.1 that got installed today.

ERROR in node_modules\@angular\platform-browser\animations\animations.d.ts(37,22): error TS-996002: Appears in the NgModule.imports of MaterialDesignModule, but could not be resolved to an NgModule class
@lucienbertin lucienbertin mentioned this issue Nov 8, 2019
0 of 4 tasks complete
@JonWallsten

This comment has been minimized.

Copy link
Contributor Author

@JonWallsten JonWallsten commented Nov 8, 2019

@alxhub: I guess you're busy. But I just wanted to follow up and make sure you got the invites to the repos.

@IgorMinar IgorMinar modified the milestones: needsTriage, v9-candidates Nov 9, 2019
alxhub added a commit to alxhub/angular that referenced this issue Nov 15, 2019
Previously, the compiler assumed that all TS files logically within a
project existed under one or more "root directories". If the TS compiler
option `rootDir` or `rootDirs` was set, they would dictate the root
directories in use, otherwise the current directory was used.

Unfortunately this assumption was unfounded - it's common for projects
without explicit `rootDirs` to import from files outside the current
working directory. In such cases the `LogicalProjectStrategy` would attempt
to generate imports into those files, and fail. This would lead to no
`ReferenceEmitStrategy` being able to generate an import, and end in a
compiler assertion failure.

This commit introduces a new strategy to use when there are no `rootDirs`
explicitly present, the `RelativePathStrategy`. It uses simpler, filesystem-
relative paths to generate imports, even to files above the current working
directory.

Fixes angular#33659
Fixes angular#33562
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 18, 2019
Previously, the compiler assumed that all TS files logically within a
project existed under one or more "root directories". If the TS compiler
option `rootDir` or `rootDirs` was set, they would dictate the root
directories in use, otherwise the current directory was used.

Unfortunately this assumption was unfounded - it's common for projects
without explicit `rootDirs` to import from files outside the current
working directory. In such cases the `LogicalProjectStrategy` would attempt
to generate imports into those files, and fail. This would lead to no
`ReferenceEmitStrategy` being able to generate an import, and end in a
compiler assertion failure.

This commit introduces a new strategy to use when there are no `rootDirs`
explicitly present, the `RelativePathStrategy`. It uses simpler, filesystem-
relative paths to generate imports, even to files above the current working
directory.

Fixes angular#33659
Fixes angular#33562
alxhub added a commit to alxhub/angular that referenced this issue Nov 18, 2019
Previously, the compiler assumed that all TS files logically within a
project existed under one or more "root directories". If the TS compiler
option `rootDir` or `rootDirs` was set, they would dictate the root
directories in use, otherwise the current directory was used.

Unfortunately this assumption was unfounded - it's common for projects
without explicit `rootDirs` to import from files outside the current
working directory. In such cases the `LogicalProjectStrategy` would attempt
to generate imports into those files, and fail. This would lead to no
`ReferenceEmitStrategy` being able to generate an import, and end in a
compiler assertion failure.

This commit introduces a new strategy to use when there are no `rootDirs`
explicitly present, the `RelativePathStrategy`. It uses simpler, filesystem-
relative paths to generate imports, even to files above the current working
directory.

Fixes angular#33659
Fixes angular#33562
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 19, 2019
Previously, the compiler assumed that all TS files logically within a
project existed under one or more "root directories". If the TS compiler
option `rootDir` or `rootDirs` was set, they would dictate the root
directories in use, otherwise the current directory was used.

Unfortunately this assumption was unfounded - it's common for projects
without explicit `rootDirs` to import from files outside the current
working directory. In such cases the `LogicalProjectStrategy` would attempt
to generate imports into those files, and fail. This would lead to no
`ReferenceEmitStrategy` being able to generate an import, and end in a
compiler assertion failure.

This commit introduces a new strategy to use when there are no `rootDirs`
explicitly present, the `RelativePathStrategy`. It uses simpler, filesystem-
relative paths to generate imports, even to files above the current working
directory.

Fixes angular#33659
Fixes angular#33562
@alxhub alxhub closed this in 850aee2 Nov 19, 2019
alxhub added a commit that referenced this issue Nov 19, 2019
鈥3828)

Previously, the compiler assumed that all TS files logically within a
project existed under one or more "root directories". If the TS compiler
option `rootDir` or `rootDirs` was set, they would dictate the root
directories in use, otherwise the current directory was used.

Unfortunately this assumption was unfounded - it's common for projects
without explicit `rootDirs` to import from files outside the current
working directory. In such cases the `LogicalProjectStrategy` would attempt
to generate imports into those files, and fail. This would lead to no
`ReferenceEmitStrategy` being able to generate an import, and end in a
compiler assertion failure.

This commit introduces a new strategy to use when there are no `rootDirs`
explicitly present, the `RelativePathStrategy`. It uses simpler, filesystem-
relative paths to generate imports, even to files above the current working
directory.

Fixes #33659
Fixes #33562

PR Close #33828
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can鈥檛 perform that action at this time.