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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apps cannot build with AOT if a dependent library was built with barrels #23713

Closed
wardbell opened this issue May 4, 2018 · 13 comments

Comments

Projects
None yet
@wardbell
Copy link
Contributor

commented May 4, 2018

This issue describes mysterious build failures for apps that depend on an npm-installed library that was built with barrels.

A barrel is an index.ts file that consolidates exported symbols of several nested files.

The problem lies somewhere in the interaction among barrels, AOT, and ngPackagr.

Workaround: Do not use barrels in your library. Always and everywhere make direct references to the source files.

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

ng build --AOT of an app fails with absurd errors concerning a library installed with npm.

That library was built with barrels.

Specifically, the top-level index.ts that tells ngPackagr which artifacts to include in the library's public API was built with barrels.

The library author (me) was able to build that library for production without any warnings or errors.
Worse, it seemed to work when used in most applications that installed the library.

But occasionally you'd get weird errors when you ng build --aot the application, such as

ERROR in ./src/client/app/test-users/test-users.module.ngfactory.js
Module not found: Error: Can't resolve 'ngrx-data/selectors/index' in '/Users/Ward/_git/sp-x/src/client/app/test-users'

Note the reference in the error to index in 'ngrx-data/selectors/index'. That's a barrel file the wasn't directly exposed publicaly.

ERROR in app/store/app-pluralizer.ts(17,13): Error during template compile of 'AppPluralizer'
  Only initialized variables and constants can be referenced in decorators because the value of this variable is needed by the template compiler in 'PLURAL_NAMES_TOKEN'
    'PLURAL_NAMES_TOKEN' references 'PLURAL_NAMES_TOKEN'
      'PLURAL_NAMES_TOKEN' references 'PLURAL_NAMES_TOKEN'
        'PLURAL_NAMES_TOKEN' is not initialized at ../../ngrx-data/utils/interfaces.ts(13,22).

The constant was initialized ... on line 16 in ngrx-data/utils/interfaces.ts.
Notice the line number is 13 in the error message. Thats where the token is declared in the d.ts type definition file (interfaces.d.ts), not the .ts source file.

Shuffling the order of exports in the top level index.ts would fix one use case but trigger another.

These are all clues that you are using barrels and AOT + ngPackagr` do not like barrels.

Expected behavior

I think we should be able to use barrels.

AOT + ngPackagr should be able to determine the proper dependency order as is possible with JIT compilation.

Minimal reproduction of the problem with instructions

There is no easy way to reproduce this bad behavior because barrels often appear to work. The problem likely arises when a library's sub-files refer to files that are exported in other barrels (even when those references are direct to the dependent files, by-passing barrels).

You can see the horror story play-out in the 1.0.0-beta.12 and 1.0.0-beta.13 releases of https://github.com/johnpapa/angular-ngrx-data.

This repo has both a library (/lib) and an example that uses that library (/src/client).

To experience the problem:

  • clone https://github.com/johnpapa/angular-ngrx-data
  • go back to the Beta 12 commit
  • npm install
  • npm run build-all # this should succeed.
  • Edit lib/src/index.ts
  • Move export * from './utils'; to the bottom of the file
  • npm run build-lib # rebuilds the library
  • ng build --aot # now it fails with the strange InjectionToken error ^^^

You can build with AOT if you remove the following providers from src/client/app/store/entity-store.module.ts

    { provide: Logger, useClass: AppLogger },
    { provide: Pluralizer, useClass: AppPluralizer }

This succeeds because these providers reference the library's Logger and Pluralizer abstract classes in ./util (the barrel).
Removing them by-passes the library reference lookup that produces the errors.

Pragmatically, this means that the library author (me) won't discover there is a problem until some consumer (you) tries to override the default logger and pluralizer behaviors by supplying alternative implementations through this official extension point.

  • revert change to lib/src/index.ts.
  • npm run build-lib # rebuilds the library
  • ng build --aot # build succeeds

A dev build (e.g, ng serve) works fine at any point in this process.

If you now move forward to Beta 13 where barrels are eliminated, all of these problems go away and you can build the sample app with ng build --aot at any point.

What is the motivation / use case for changing the behavior?

Barrels nicely encapsulate and minimize low-level details of a library's internal folder structure.
They work great with JIT. Obviously not with AOT at this time.

Environment


Angular version: 5.2.10
ngPackagr version: 2.4.2

I do not know if it is still a problem in v.6. I don't have the time to find out.

Browser: Irrelevant

For Tooling issues: irrelevant

@wardbell

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2018

Feel free to close this issue if we do not have time or the intention of fixing it.

I've entered it here primarily so that other library builders (a) can understand why consumers of their libraries are getting strange errors when they build their apps for production and (b) solve that problem relatively quickly by eliminating barrels from their libraries.

@wardbell wardbell changed the title Bug: apps cannot build with AOT if a dependent library was built with barrels Apps cannot build with AOT if a dependent library was built with barrels May 4, 2018

@ngbot ngbot bot added this to the needsTriage milestone May 10, 2018

@ngbot ngbot bot modified the milestones: needsTriage, Backlog May 24, 2018

@piernik

This comment has been minimized.

Copy link

commented Jun 13, 2018

The same issue here - problem with index.ts files.
Here is my angular-cli issue with sample project with isolated bug: angular/angular-cli#11210

@piernik

This comment has been minimized.

Copy link

commented Jun 19, 2018

Any news on this one? Or should we refactor our code? When it will be fixed?

@jasonaden

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

I'm following up on this with the team to see what clarification we can get around this issue. Will post an update next week.

@admosity

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2018

There is an open PR that looks to resolve this issue by @jeffora: #22856

@jasonaden @filipesilva @IgorMinar Can we get someone to look at this?

Relevant issue in ng-packagr:
ng-packagr/ng-packagr#195

Basically the metadata files are exported without the actual metadata because it doesn't know where to find the correct file to generate the metadata from. Here is a metadata file generated from a large library that uses barrels (with angular cli orchestrating ng-packagr):
image

Most of these issues prop up in AOT because AOT depends on these metadata files. As shown below with ng build --prod with angular cli:

image

But works perfectly fine in non AOT:
image

It seems there's a number of issues with barrels and metadata not being found as well:
angular/angular-cli#8581
angular/angular-cli#7066
#24225
#22825

Looks like most of this will be fixed in Ivy with doing away with metadata.json, but will there be something in 6? Noticed that tag that @mhevery put in #24225

@jeffora

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2018

I opened my PR for this before 6 left beta, in the hope of getting it included (and possibly backported to 5.x). However, the PR has been completely ignored for months despite numerous requests for review / comment. I can only assume the team doesn't view this as a high priority issue.

@jasonaden

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2018

@jeffora Thanks for the PR and it was overlooked, though it's definitely an important one. I'm closing this issue as that PR has now been merged. It should be out with tomorrow's release. #22856

@jasonaden jasonaden closed this Jul 10, 2018

@jeffora

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2018

@jasonaden Thanks, and no worries. Glad to see it get in!

@admosity

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2018

Thanks @jeffora for saving us hours of unbarreling work!

@Thorski

This comment has been minimized.

Copy link

commented Aug 30, 2018

It seems this exact same problem occurs with Angular 6. Angular cli libraries.
I had to remove all my references to barrels to make it work.

Angular CLI: 6.1.5
Angular: 6.1.4

@dherges

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

TL;DR Barrels are full of oil. We want green Angular! 😆

rovico pushed a commit to sunrise-r/interface-admin-ng that referenced this issue Mar 20, 2019

@jonathanlie

This comment has been minimized.

Copy link

commented Jun 6, 2019

I'm still having this issue in Angular 8. In a library, anything with decorator won't work if exported using barrel export.
Any resolution to this issue, or perhaps another issue I can keep track of?

@robertsmith2

This comment has been minimized.

Copy link

commented Jun 13, 2019

I am in the same spot as @jonathanlie this is still an issue in Angular 8 using a library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.